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

Remove rigid body data structure from Compound #1203

Merged
merged 6 commits into from
Oct 30, 2024

Conversation

chrisjonesBSU
Copy link
Contributor

@chrisjonesBSU chrisjonesBSU commented Oct 28, 2024

PR Summary:

@CalCraven and I have recently been removing the writers from mBuild now that most of them are in GMSO, and during that time some discussion came up about how to handle rigid bodies. Should we keep them in mBuild then read them into GMSO when converting from a compound to a topology, or should all rigid body assignment and handling only be done in GMSO? The only use case for rigid bodies in mBuild were in the HOOMD related writers, and now that they live exclusively in GMSO, we felt it was best to handle rigid body assignment purely in GMSO (See PR #850)

This PR completely removes the rigid body data structure and related methods from compound.py and any rigid body assignment done in conversion.py

PR Checklist


  • Includes appropriate unit test(s)
  • Appropriate docstring(s) are added/updated
  • Code is (approximately) PEP8 compliant
  • Issue(s) raised/addressed?

@chrisjonesBSU chrisjonesBSU linked an issue Oct 28, 2024 that may be closed by this pull request
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.

One small change, then we have to see why the 3.10 test is failing

mbuild/tests/base_test.py Show resolved Hide resolved
@chrisjonesBSU
Copy link
Contributor Author

@Zeerakkhan47 I know we discussed having you attempt to make this PR on the last dev meeting, but we felt it was important to get this one done before the 1.0 release, so I wanted to get it done ASAP. Please feel free to review the changes and raise any issues or questions!

Copy link

codecov bot commented Oct 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.73%. Comparing base (38077cd) to head (a4c41c5).
Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1203      +/-   ##
==========================================
- Coverage   87.35%   85.73%   -1.63%     
==========================================
  Files          62       53       -9     
  Lines        6588     4802    -1786     
==========================================
- Hits         5755     4117    -1638     
+ Misses        833      685     -148     

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

@chrisjonesBSU chrisjonesBSU merged commit ba89d6c into mosdef-hub:main Oct 30, 2024
20 of 21 checks passed
@chrisjonesBSU chrisjonesBSU deleted the remove-rigid branch October 30, 2024 21:23
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.

Remove rigid body ID assignment from Compound
2 participants