Replace-species#2291
Conversation
pymatgen/core/structure.py
Outdated
| return False | ||
| for site in self: | ||
| if site not in other: | ||
| def __eq__(self, other: object) -> bool: |
There was a problem hiding this comment.
Please do not make this change. I am unclear why your PR is mentioning a replace method but you are modifying the way structure checks for equality.
There was a problem hiding this comment.
Sorry, this wasn't meant to be bunched in with the other small changes in this PR. I'll revert it.
In general, updating all Pymatgen rich comparison functions to send NotImplemented follows recommended practice. See https://docs.python.org/3/library/constants.html#NotImplemented. So maybe worth tackling in a separate PR.
There was a problem hiding this comment.
The PR mentioned the Structure.replace_species() method cause my original intent was just to update its doc string to make it clearer it makes in-place modifications.
|
Thanks @janosh, I see you reverted the other commit so going to merge this |
4 participants
Structure().replace_speciesdoc string should make it clear that original structure is modified in place.