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.

Sign up for GitHub

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

Jump to bottom

numpy-ify doctstrings #339

Closed
4 tasks done
bpoldrack opened this issue Apr 25, 2023 · 3 comments
Closed
4 tasks done

numpy-ify doctstrings #339

bpoldrack opened this issue Apr 25, 2023 · 3 comments

Comments

@bpoldrack
Copy link
Collaborator

bpoldrack commented Apr 25, 2023 •
edited by TobiasKadelka
Loading

In order to have

  • consistent, easy to read docstring when actually looking at source code files, as well as
  • proper rendering of developer docs

we should decide on a style that works for us. Generally, I like the numpy style. With type hints being implemented already, it would be nice to figure whether this can easily be rendered and linted if we simply leave out the types in the docstrings.

Generally, actually rendering developer docs is a different task, but should probably be considered in conjunction with this issue.

  • explore how to not make us write redundant type hints
  • decide on a style
  • document style in README (or a CONTRIBUTING doc)
  • actually do it
TobiasKadelka added a commit to TobiasKadelka/onyo that referenced this issue Jul 21, 2023
@TobiasKadelka
TST/DOC: add test for OnyoRepo.clear_caches()
1193a15 
Adds a docstring in the style discussed in psyinfra#339.
This test modifies an asset in OnyoRepo with an external function
so that the cache is not up-to-date, and then tests the function
`OnyoRepo.clear_caches()` to re-load the cache.
TobiasKadelka added a commit to TobiasKadelka/onyo that referenced this issue Jul 21, 2023
@TobiasKadelka
TST/DOC: add test and docstring to OnyoRepo.generate_commit_message()
fd7a790 
Gives the function a new docstring in the style discussed in issue psyinfra#339.
The test modifies an OnyoRepo with the python implementations of the
commands and then calls `generate_commit_message()` and checks the
default commit message that was generated.
TobiasKadelka added a commit to TobiasKadelka/onyo that referenced this issue Jul 25, 2023
@TobiasKadelka
TST/DOC: add test and docstring to OnyoRepo.generate_commit_message()
6a4ccc9 
Gives the function a new docstring in the style discussed in issue psyinfra#339.
The test modifies an OnyoRepo with the python implementations of the
commands and then calls `generate_commit_message()` and checks the
default commit message that was generated.

There is the idea to change the commit message body in a future PR.
This would impact parts of the test function written now, and I assume
the current test should help as a starting point to test the new format.
TobiasKadelka added a commit to TobiasKadelka/onyo that referenced this issue Jul 25, 2023
@TobiasKadelka
DOC: update docstring to OnyoRepo.__init__() with new docstring format
3250310 
Gives the function a new docstring in the style discussed in issue psyinfra#339.
@TobiasKadelka TobiasKadelka mentioned this issue Jul 25, 2023
Merged
TobiasKadelka added a commit to TobiasKadelka/onyo that referenced this issue Jul 26, 2023
@TobiasKadelka
DOC: update docstring and type hints to OnyoRepo.__init__()
185c417 
Gives the function a new docstring in the style discussed in issue psyinfra#339.
Also normalizes type hints e.g. usage of `Optional`, and other code style aspects.
TobiasKadelka added a commit to TobiasKadelka/onyo that referenced this issue Jul 26, 2023
@TobiasKadelka
TST/DOC: add test for OnyoRepo.clear_caches()
404c610 
Adds a docstring in the style discussed in psyinfra#339.
This test modifies an asset in OnyoRepo with an external function
so that the cache is not up-to-date, and then tests the function
`OnyoRepo.clear_caches()` to re-load the cache.
TobiasKadelka added a commit to TobiasKadelka/onyo that referenced this issue Jul 26, 2023
@TobiasKadelka
TST/DOC: add test and docstring to OnyoRepo.generate_commit_message()
7899aa8 
Gives the function a new docstring in the style discussed in issue psyinfra#339.
The test modifies an OnyoRepo with the python implementations of the
commands and then calls `generate_commit_message()` and checks the
default commit message that was generated.

There is the idea to change the commit message body in a future PR.
This would impact parts of the test function written now, and I assume
the current test should help as a starting point to test the new format.
@TobiasKadelka TobiasKadelka mentioned this issue Jul 28, 2023
Merged
TobiasKadelka added a commit to TobiasKadelka/onyo that referenced this issue Jul 31, 2023
@TobiasKadelka
TST/DOC: add test and docstring to OnyoRepo.generate_commit_message()
484aa43 
Gives the function a new docstring in the style discussed in issue psyinfra#339.
The test modifies an OnyoRepo with the python implementations of the
commands and then calls `generate_commit_message()` and checks the
default commit message that was generated.

There is the idea to change the commit message body in a future PR.
This would impact parts of the test function written now, and I assume
the current test should help as a starting point to test the new format.
@aqw
Copy link
Collaborator

aqw commented Oct 20, 2023

In the absence of a strong opinion, I like going with your experience. :-)

It looks like the numpy style is already introduced to the codebase, so I'd say it's fairly well decided to use it.

Regarding the type hints, there's a numpy issue about this (of course, Yarik has already weighed in ;-). There's no resolution, but some have success with sphinx-autodoc-typehints.

@aqw aqw changed the title Decide on a docstring style numpy-ify doctstrings Oct 20, 2023
@aqw
Copy link
Collaborator

aqw commented Oct 20, 2023

I have updated the issue title. If you feel that was premature, please share your objections. :-)

aqw added a commit that referenced this issue Oct 20, 2023
@aqw
DOC: declare numpy as the docstring style
8676da3 
#339
@aqw aqw mentioned this issue Oct 20, 2023
Merged
@TobiasKadelka
Copy link
Collaborator

As far as I see, we decided to go redundant with the type hints after all, and begin applying it to all functions we touch.

Since we have more specific issues regarding documentation/docstrings, I close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@aqw @bpoldrack @TobiasKadelka