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

Add docstrings to scripts #80

Merged
merged 5 commits into from
Jan 27, 2020
Merged

Add docstrings to scripts #80

merged 5 commits into from
Jan 27, 2020

Conversation

smoia
Copy link
Member

@smoia smoia commented Dec 6, 2019

Closes #1 .
Added docstrings for utils.py and viz.py.

Are we happy with the docstrings or should we expand more main() in phys2bids.py?

@smoia smoia added Documentation This issue or PR is about the documentation Urgent If you don't know where to start, start here! labels Dec 6, 2019
@smoia smoia requested review from rmarkello and RayStick and removed request for rmarkello December 6, 2019 15:58
@smoia smoia added this to the OHBM poster milestone Dec 6, 2019
@rmarkello
Copy link
Member

I'm happy to go through and review this but I have a quick question before getting started: are we trying to follow numpydoc conventions? If so, I'll comment on all the places where the docs are currently not doing that, but I didn't want to do that without checking!

@smoia
Copy link
Member Author

smoia commented Dec 6, 2019

I answer to your question with a question: what is the numpydoc convention for outputs (as in: files written to disk)?

@rmarkello
Copy link
Member

There's nothing in particular to highlight that a function will save a file to disk (see, for example, np.savetxt). Having something at the top of the function doc-string to note that it's generating a file seems sufficient.

But I think, generally speaking, following numpydoc conventions can be really useful since they're a pretty widely-used standard and it would help Python users coming from other projects more quickly get acquainted with the codebase.

@smoia
Copy link
Member Author

smoia commented Dec 6, 2019

Ok, then: I'm going to momentarily close this issue, make the docstrings more numpyish, but maintain the "Outcome" for when we save something to the disk or print something on screen.

@smoia smoia closed this Dec 6, 2019
@smoia smoia mentioned this pull request Dec 10, 2019
@smoia smoia reopened this Jan 3, 2020
@smoia smoia requested a review from rmarkello January 3, 2020 15:08
@smoia smoia closed this Jan 3, 2020
@smoia smoia reopened this Jan 3, 2020
@smoia smoia changed the title Doc/docstrings Adding docstrings to scripts Jan 8, 2020
@smoia smoia requested review from eurunuela and vinferrer January 10, 2020 11:08
Copy link
Collaborator

@eurunuela eurunuela left a comment

Choose a reason for hiding this comment

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

LGTM!

@smoia smoia merged commit e0ce249 into physiopy:master Jan 27, 2020
@smoia smoia deleted the doc/docstrings branch February 6, 2020 22:51
@smoia smoia changed the title Adding docstrings to scripts Add docstrings to scripts Mar 26, 2020
@smoia smoia added the released This issue/pull request has been released. label Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation This issue or PR is about the documentation released This issue/pull request has been released. Urgent If you don't know where to start, start here!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Write docstrings
3 participants