docs: type hinting and docstrings in molstruct#497
Conversation
gcroci2
left a comment
There was a problem hiding this comment.
I left minor comments, more for me to understrand than for suggesting changes. I removed the duplicated comments, but in case we decide to change something we need to remember that it may be in multiple files.
| if isinstance (other, Atom): | ||
| return (self._residue == other._residue | ||
| and self._name == other._name) | ||
| return NotImplemented |
There was a problem hiding this comment.
Doesn't bool give errors with the NotImplemented case?
There was a problem hiding this comment.
I found this here, when researching something else (https://stackoverflow.com/questions/63503512/python-type-hinting-own-class-in-method). Not sure where I can find the formal correct way to format __eq__, but the commentors on this post all seem to agree that this is the correct way to do it.
There was a problem hiding this comment.
You could run mypy to verify that the type hints are right
Sorry, I was actually meaning to leave references to some of these changes when I made the PR, but then forgot :) I added them now in reply to your comments |
and docstring formatting
|
3 additional changes:
You should be able to go through the commits one by one to see each change in isolation, which might make reviewing easier. Note that because I have "automatic import sorting upon save" turned on, some additional imports are switched around as well. |
Regarding 3., JFYTK, we're using the Google Style convention for docstrings. Nice that you moved |
Thanks for the ref to google convention. It does still follow that as well. I wasn't sure whether the class itself should get a docstring or only the init method. But in both style guides, they each get a docstring. |
This was really bugging me while refactoring query, so did a separate extra PR for it.