Skip to content
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

Docstring compliance #323

Merged
merged 18 commits into from
Jan 16, 2025
Merged

Docstring compliance #323

merged 18 commits into from
Jan 16, 2025

Conversation

arik-shurygin
Copy link
Collaborator

@arik-shurygin arik-shurygin commented Jan 13, 2025

  • using pydocstyle to ensure method comments follow PEP257 guidelines
  • Method comment formatting to be more in line with Numpy guidelines and make comments more concise with some examples added as well.
  • Better type hints on functions that were missing them
  • Making some impure functions more pure favoring returning calculated variables as opposed to setting them in self from within the method. 

This PR will make future revamps easier and is a part of general repo maintenance in preparation for v1.0 overhaul.

Some functions were left untouched because they will likely be drastically changed by GH issues like #319 #321 #318 or are soon to be deprecated and removed.

CLOSES #313

@arik-shurygin
Copy link
Collaborator Author

arik-shurygin commented Jan 13, 2025

bringing back into drafts to implement pydocstyle on all this

@arik-shurygin arik-shurygin marked this pull request as ready for review January 15, 2025 17:18
Copy link
Collaborator

@edbaskerville edbaskerville left a comment

Choose a reason for hiding this comment

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

Just a few unimportant copyedits. I'm not sufficiently familiar with numpydoc to assess this properly.

Propose adding numpydoc check to the linter, which as discussed looks like we can just do with the --convention=numpy flag?

Copy link

@cdc-ap66 cdc-ap66 left a comment

Choose a reason for hiding this comment

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

This looks great and I'm sure it was tedious to go through. Thank you for setting a great example :). LGTM

next step will be to tackle the format of the docstringed Examples and to add pydocstring to pre-commit

@arik-shurygin arik-shurygin merged commit c752b46 into main Jan 16, 2025
4 checks passed
@arik-shurygin arik-shurygin deleted the docstring-compliance branch January 16, 2025 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ensure all docstrings are numpy format
3 participants