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

DOC: Misc numpydoc syntax fixing. #3205

Merged
merged 1 commit into from
Apr 24, 2021
Merged

Conversation

Carreau
Copy link
Contributor

@Carreau Carreau commented Apr 17, 2021

This uses https://github.com/Carreau/velin to try to automatically fix a
couple of issues in docstrings to match the numpydoc syntax.

  • Space before colon in Parameter sections is necessary for numpydoc to
    properly recognize parameter name and type.
  • Naming of sections in numpydoc is plural usually

This also remove a couple of documented parameters that don't exists
(either removed, or copy-past is my guess).

This uses https://github.com/Carreau/vélin to try to automatically fix a
couple of issues in docstrings to match the numpydoc syntax.

 - Space before colon in Parameter sections is necesary for numpydoc to
 properly recognize parameter name and type.
 - Naming of sections in numpydoc is plural usually

This also remove a couple of documented parameters that don't exists
(either removed, or copy-past is my guess).
@welcome
Copy link

welcome bot commented Apr 17, 2021

Hi! Welcome, and thanks for opening this pull request. We have some guidelines for new pull requests, and soon you'll hear back about the results of our tests and continuous integration checks. Thank you for your contribution!

@neutrinoceros
Copy link
Member

Very cool, thanks Matthias !
I see that Velin exposes a pre-commit hook as well, which is awesome. Would you mind adding it to our .pre-commit-config.yaml ?

@neutrinoceros neutrinoceros added docs enhancement Making something better labels Apr 21, 2021
Copy link
Member

@munkm munkm left a comment

Choose a reason for hiding this comment

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

This is great! Thanks Matthias!!!!

@munkm
Copy link
Member

munkm commented Apr 21, 2021

Maybe we can open a "good first contribution" issue to add the pre-commit hook! That would be a nice fix for a new contributor. What do you think @neutrinoceros ?

@neutrinoceros
Copy link
Member

I think it'd take much less time to do it myself TBF. I'm giving priority to Matthias here as courtesy to the author :)

@munkm
Copy link
Member

munkm commented Apr 21, 2021

I think it'd take much less time to do it myself TBF.

That's the nature of all "good first contribution" issues 🙂

@neutrinoceros
Copy link
Member

True. I also don't think we should advertise "solve our infrastructure" as good first issues. Most potential contributors wouldn't know what to do, and the others could question our ability to do the job.

@munkm
Copy link
Member

munkm commented Apr 21, 2021

If somebody questions our ability to do our job because we're trying to build community with good, technical, not demeaning "good first contribution" labels on issues that's their problem, not ours.

@Carreau
Copy link
Contributor Author

Carreau commented Apr 23, 2021

I see that Velin exposes a pre-commit hook as well, which is awesome. Would you mind adding it to our .pre-commit-config.yaml ?

I think it is not yet stable enough to be added as a an alway on pre-commit; there are still bugs, like for example if you forget the blank line before the Return section, because of how numpydoc works it will just nuke the section.

There are also case where it get things wrong like:

Yields
------

A tuple of :class:`FooBar`

It will replace to : class:`FooBar` which is invalid RST syntax.

So maybe a bit later, when I'm more confident that it won't just throw away a paragraph you took 10 minutes to write :-)

@neutrinoceros
Copy link
Member

Totally fine by me. In that case we can merge this as in AFAIC. Thanks again !

@neutrinoceros neutrinoceros merged commit 83b4ddf into yt-project:main Apr 24, 2021
@welcome
Copy link

welcome bot commented Apr 24, 2021

Hooray! Congratulations on your first merged pull request! We hope we keep seeing you around! 🎆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs enhancement Making something better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants