Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Pydantic v1 and v2 #752

Merged
merged 3 commits into from
Aug 11, 2023
Merged

Conversation

mattwthompson
Copy link
Member

@codecov
Copy link

codecov bot commented Aug 1, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.05% 🎉

Comparison is base (0b9e0c5) 92.02% compared to head (fd21301) 92.07%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #752      +/-   ##
==========================================
+ Coverage   92.02%   92.07%   +0.05%     
==========================================
  Files          67       67              
  Lines        6873     6892      +19     
==========================================
+ Hits         6325     6346      +21     
+ Misses        548      546       -2     
Files Changed Coverage Δ
gmso/abc/abstract_connection.py 97.91% <100.00%> (+0.04%) ⬆️
gmso/abc/abstract_potential.py 90.41% <100.00%> (+0.13%) ⬆️
gmso/abc/abstract_site.py 94.73% <100.00%> (+0.07%) ⬆️
gmso/abc/auto_doc.py 27.60% <100.00%> (+0.44%) ⬆️
gmso/abc/gmso_base.py 97.43% <100.00%> (+0.03%) ⬆️
gmso/core/angle.py 96.66% <100.00%> (+0.11%) ⬆️
gmso/core/angle_type.py 100.00% <100.00%> (ø)
gmso/core/atom.py 91.89% <100.00%> (+0.11%) ⬆️
gmso/core/atom_type.py 98.70% <100.00%> (+1.33%) ⬆️
gmso/core/bond.py 100.00% <100.00%> (ø)
... and 9 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mattwthompson
Copy link
Member Author

It looks like this sort of change would need to be propagated through the entire stack all at once

@daico007
Copy link
Member

daico007 commented Aug 1, 2023

There was some additional change introduced in pydantic v1.10.12 that failed our set up, I haven't checked, but those changes probably also causing the failing tests here.

@mattwthompson
Copy link
Member Author

It originally launched with a broken build (conda-forge/admin-requests#770 begins the paper trail) but was fixed. There could be other issues, but the one I ran into is not what I'm seeing in these logs

@CalCraven
Copy link
Contributor

Yeah the issue is just forcefield-utilities. I think we can quickly release get Pydantic 2 as a PR there and release it so these tests pass.
Looks like pydantic is in gmso_xml.py and foyer_xml.py only, so that can be done.

@mattwthompson
Copy link
Member Author

I've done the absolute minimum work to get that going: mosdef-hub/forcefield-utilities#36

Copy link
Contributor

@CalCraven CalCraven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably good as a stopgap here.

@mattwthompson
Copy link
Member Author

Is there a path to getting tests passing? The first thing I'd try, short of a release of forcefield-utilities, is just installing its development version

@CalCraven
Copy link
Contributor

@daico007 merged the PR into ffutils, so we can rerun the tests here and see if the bleeding edge tests pass.

@mattwthompson
Copy link
Member Author

Ah, nice, I forgot you all set up those tests. Should be useful!

@mattwthompson
Copy link
Member Author

Cool, looks like that run passed: https://github.com/mosdef-hub/gmso/actions/runs/5729710869/job/15560739904

Copy link
Member

@daico007 daico007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the fix Matt! I will probably release a patch for forcefield-utilities tomorrow, follow with a patch for GMSO to have this new fix in.

@mattwthompson
Copy link
Member Author

Happy to help - could you take on the responsibility of merging this when appropriate?

No rush on a release but will be happy to see it out

@daico007
Copy link
Member

daico007 commented Aug 2, 2023

Yes, for sure, I am planning to have this merged right before releasing (just so the tests on the main CI doesn't appear half-failed), but I could merge this now if you need this for other work.

@mattwthompson
Copy link
Member Author

Not urgent at all, this won't cause us major problems for a few months

@daico007 daico007 merged commit 525cfd3 into mosdef-hub:main Aug 11, 2023
@mattwthompson mattwthompson deleted the pydantic-1-2 branch August 11, 2023 18:16
emarinri pushed a commit to emarinri/gmso that referenced this pull request Aug 13, 2023
daico007 added a commit that referenced this pull request Sep 22, 2023
* Add fixed length bonds and angles to potential templates library.

* Working to make MCF writer feature complete. Tests in progress.

* Add fixed angle support
* Fix charge sign error (gmso-wide)
* Update ring identification to handle fused systems (mbuild #744)
* Increase floating point accuracy
* Increase element type length and atom type length
* Correctly write atom indices for bonds/angles/dihedrals/impropers

* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci

* Add dimensions to Fixed Bond and Angle templates

* Update keys in OPLS dihedral parameters dict

* Modify dihedral constant indexing from k0 to k3
to k1 to k4 in the OPLSTorsionPotential template.

* Fix typed OPLS ethane test for MCF format

* Updated from old Mie Xenon test to OPLS ethane.
* Remove assertions related to n and m parameters of the Mie potential.
* Fix floating point format issues in MCF writer.

* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci

* Add typed ethane test.

This test is not complete yet. The 1/2 factor
in the angles and OPLS dihedral is not
accounted for.

* Fix code style

* Add fixture to parse mcf into sections

* Support Pydantic v1 and v2 (#752)

Co-authored-by: Co Quach <[email protected]>

* Update MCF with GMSO builtin top site sorting

* Use PotentialFilters to check for unique atomtypes

* Add test to check charge neutrality

* Add test for incompatible expressions

* Test full MCF for Mie-Xe and LJ-Ar

* Check charge neutrality in each molecule test

* Add exception for not neutral system in MCF writer

* Move parsing and neutrality check as utils

* minor docstring/comment fixes, swap out simplify with symengine expand

* [pre-commit.ci] pre-commit autoupdate (#763)

updates:
- [github.com/psf/black: 23.7.0 → 23.9.1](psf/black@23.7.0...23.9.1)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Test for 0.5 factor of OPLS dihedral potential

* Test to account for 0.5 factor in harmonic angles

* Test for rigid angles using the TIP3P model

* MCF writer test with topology with 10 argon mols

* Tentative output of multiple MCF from one Topology

* Change psi to phi and consts for ethylene xml test

* Test for 0.5 factor in OPLS dihedrals

* Add cassandra test for gmso

* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci

* Add test for two atom fragment

* Test for molecule with one ring

* Change nitrogen test to ethane ua

* Add ethane rigid reference xml

* Use Fourier dihedrals as MCF standard dihedrals.

* Fix dihedral type output fourier to opls

* Match MCF GMSO Angle header to mb Angle header

* Add a test comparing gmso and mbuild mcf writers

* Use the Fourier converter for ethylene dihedrals

* Write atom type masses instead of atom masses to account for UA beads

* Output correct case for dihedral styles in MCFs

* Run an energy calculation to compare GMSO and mBuild MCF writers

* Update gmso/tests/test_mcf.py

* Update gmso/tests/test_mcf.py

---------

Co-authored-by: Ryan S. DeFever <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Matt Thompson <[email protected]>
Co-authored-by: Co Quach <[email protected]>
Co-authored-by: Co Quach <[email protected]>
Co-authored-by: CalCraven <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants