mosdef-hub / mbuild

A hierarchical, component based molecule builder
https://mbuild.mosdef.org
Other
171 stars 79 forks source link

Add option to reset child labels in Compound.remove() #1173

Closed jaclark5 closed 4 months ago

jaclark5 commented 5 months ago

Two changes are made: 1) Add keyword argument to Compound.remove() for resetting the labels to achieve two goals. First, if all children of some category are removed, the parent category label will inherently be removed, while the ports are also renumbered. 2) In Compound.add() if a single Compound is added without a label, the Compound.name attribute is used instead of Compound.class.name to align with the default behavior in:

PR Summary:

Resolves #1172

Using the code in the issue where the last line is now, Molecule.remove(remove_array, reset_labels=True), the results are now:

Before ['C', 'C[0]', 'H', 'H[0]', 'H[1]', 'C[1]', 'H[2]', 'H[3]', 'O', 'O[0]', 'down', 'up']
After Flatten ['monomer', 'H', 'C', 'C[0]', 'H[2]', 'H[3]', 'C[1]', 'H[4]', 'H[5]', 'O', 'O[0]', 'H[6]', 'H[7]']
After Remove H ['C', 'C[0]', 'C[1]', 'O', 'O[0]', 'port', 'port[0]', 'port[1]', 'port[2]', 'port[3]', 'port[4]', 'port[5]']

After

PR Checklist


codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 87.32%. Comparing base (df74ac1) to head (d9c2cfa).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1173 +/- ## ========================================== + Coverage 87.28% 87.32% +0.04% ========================================== Files 62 62 Lines 6502 6525 +23 ========================================== + Hits 5675 5698 +23 Misses 827 827 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

CalCraven commented 4 months ago

We should potentially add in a future PR a deprecation warning for reset_index