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

Update sphinxcontrib-napoleon to sphinx.ext.napoleon #65

Merged
merged 1 commit into from
Oct 28, 2022

Conversation

ianyfan
Copy link
Contributor

@ianyfan ianyfan commented Oct 27, 2022

sphinxcontrib-napoleon is obsolete and also breaks with Python 3.10

napoleon_google_docstring = True
napoleon_numpy_docstring = False

are defaults in sphinx.ext.napoleon so I removed them (but there may still be value in explicitly mentioning them)

The only remaining reference is a link in the README to an example of style guide: https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html but if there is a better example that could also be replaced

@Rocamonde
Copy link
Member

Thanks for doing this. I think that removing defaults makes sense. For the remaining link, we could use https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings

@ianyfan
Copy link
Contributor Author

ianyfan commented Oct 27, 2022

That link is already present, the napoleon link is given as an example

Copy link
Member

@AdamGleave AdamGleave left a 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 after updating link + tests passing.

For the link, what about https://www.sphinx-doc.org/en/master/usage/extensions/example_google.html -- I just googled for "sphinx example google style python docstrings" and it came up ;), but looks official.

@codecov
Copy link

codecov bot commented Oct 28, 2022

Codecov Report

Merging #65 (5b2cbcf) into master (3d2cd41) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master       #65   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        26           
  Lines          982       982           
=========================================
  Hits           982       982           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ianyfan
Copy link
Contributor Author

ianyfan commented Oct 28, 2022

Updated commit with the new link, thanks

@ianyfan ianyfan merged commit eeb9beb into HumanCompatibleAI:master Oct 28, 2022
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.

3 participants