Skip to content
New issue

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

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

Already on GitHub? # to your account

Add tests and custom exceptions for index lookups in Topology #1302

Merged
merged 1 commit into from
May 20, 2022

Conversation

mattwthompson
Copy link
Member

@mattwthompson mattwthompson commented May 19, 2022

While working on #1276, I found that Topology.molecule_index was only ever used in the machinery that checks which molecules had charges assigned to them. I added a couple of unit tests, custom exceptions, and updated some docstrings.

I noticed there are a lot of bits in the Topology API that deal with particles, i.e.

    @property
    def particles(self):
        for molecule in self.molecules:
            for atom in molecule.atoms:
                yield atom

@j-wags would you accept a PR that removes or deprecates the concept of a "particle" since virtual sites are no longer a part of Molecule objects?

@codecov
Copy link

codecov bot commented May 19, 2022

Codecov Report

Merging #1302 (55c5e15) into topology-biopolymer-refactor (6001482) will increase coverage by 0.53%.
The diff coverage is 100.00%.

@j-wags
Copy link
Member

j-wags commented May 19, 2022

@j-wags would you accept a PR that removes or deprecates the concept of a "particle" since virtual sites are no longer a part of Molecule objects?

Oh, good question. Yes, I think that the 0.11.0 release would be a good time to give particles the same treatment as topology_atoms - Have the particles API points instead redirect to atoms and issue a deprecation warning that they'll be completely gone in 0.12.

@mattwthompson mattwthompson marked this pull request as ready for review May 19, 2022 22:31
@mattwthompson mattwthompson mentioned this pull request May 19, 2022
5 tasks
Copy link
Member

@j-wags j-wags left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@mattwthompson
Copy link
Member Author

Thanks!

@mattwthompson mattwthompson merged commit 9548cd0 into topology-biopolymer-refactor May 20, 2022
@mattwthompson mattwthompson deleted the topology-index-lookups branch May 20, 2022 18:23
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants