-
Notifications
You must be signed in to change notification settings - Fork 10
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
add docstrings to all functions #100
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quite some issues, most of them about how to describe defaults in the docstrings.
Feel free to ignore them, if you don't agree with this way of mentioning defaults
src/pytom_tm/weights.py
Outdated
shape: tuple[int, int, int] | ||
shape tuple with x,y or x,y,z dimension |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this then be:
shape: tuple[int, int, int] | |
shape tuple with x,y or x,y,z dimension | |
shape: Union[tuple[int, int, int], tuple[int, int]] | |
shape tuple with x,y or x,y,z dimension |
and also be fixed in the type hint?
src/pytom_tm/weights.py
Outdated
shape: tuple[int, int, int] | ||
shape tuple with x,y or x,y,z dimension |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
shape: tuple[int, int, int] | |
shape tuple with x,y or x,y,z dimension | |
shape: Union[tuple[int, int, int], tuple[int, int]] | |
shape tuple with x,y or x,y,z dimension |
src/pytom_tm/weights.py
Outdated
shape: tuple[int, int, int] | ||
shape tuple with x,y or x,y,z dimension |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as above
shape: tuple[int, int, int] | |
shape tuple with x,y or x,y,z dimension | |
shape: Union[tuple[int, int, int], tuple[int, int]] | |
shape tuple with x,y or x,y,z dimension |
src/pytom_tm/weights.py
Outdated
shape tuple with x,y or x,y,z dimension | ||
spacing: float | ||
voxel size in real space | ||
low_pass: Optional[float] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
low_pass: Optional[float] | |
low_pass: Optional[float], default None |
src/pytom_tm/weights.py
Outdated
voxel size in real space | ||
low_pass: Optional[float] | ||
resolution of low-pass filter | ||
high_pass: Optional[float] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
high_pass: Optional[float] | |
high_pass: Optional[float], default None |
Co-authored-by: Sander Roet <[email protected]>
Thanks for the detailed review. I think its a good idea to add the defaults ! just FYI went through the first suggestions, will continue tonight or tomorrow. |
Co-authored-by: Sander Roet <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 leftover issues from the previous round, LGTM otherwise
Co-authored-by: Sander Roet <[email protected]>
Co-authored-by: Sander Roet <[email protected]>
thanks for the links, yep now they are resolved! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, feel free to merge
This is a draft PR to expand and update the docstrings of the code.
This PR expands doscstrings, except for the file correlation.py where I updated some functions to first create an output return variable. In all other files its just docstrings updates.
Closes #8
PR#100 💯