Skip to content

Fix minor docstring issues in dataclasses.py #93024

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

Merged
merged 1 commit into from
Jul 26, 2022

Conversation

romanngg
Copy link
Contributor

Fix minor docstring issues in dataclasses.py.

Otherwise, when using functools.wrap around them (and inherit their docstrings), sphinx renders the docstrings badly and raises warnings about wrong indent.

@romanngg romanngg requested a review from ericvsmith as a code owner May 20, 2022 19:26
@ghost
Copy link

ghost commented May 20, 2022

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@ericvsmith
Copy link
Member

I don’t know a lot about sphinx (clearly). Could you walk me through a few steps on how I can verify the previous behavior was wrong, and this PR fixes the issue? Thanks.

@AA-Turner
Copy link
Member

@ericvsmith this is more a reStructuredText issue than a Sphinx one -- the double colon marks the start of an (indented) code block, and the other changes seem alright in terms of ensuring that consistency of indentation is correct within the same code-block.

As for testing, perhaps @romangg is using Sphinx's autodoc or something similar, which renders the docstring as reStructuredText?

A

Otherwise, when using `functools.wrap` around them (and inherit their docstrings), sphinx renders the docstrings badly and raises warnings about wrong indent.
@romanngg
Copy link
Contributor Author

romanngg commented May 23, 2022

Thanks @AA-Turner that's exactly what I'm doing! @ericvsmith in terms of verifying I can suggest the following steps (also newbie here, haven't figured out a simpler way to repro):

  1. pip install sphinx
  2. In some directory make an index.rst file containing
.. automodule:: dataclasses
   :members:

and a conf.py file with

extensions = ['sphinx.ext.autodoc',]
  1. In that directory run sphinx-build . _build

Without the changes above, you will see warnings like

romann@romann2 ~/autodoc_test> sphinx-build . _build                                                                                                                                                                                   (base) 
Running Sphinx v4.2.0
loading pickled environment... done
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 0 source files that are out of date
updating environment: 0 added, 1 changed, 0 removed
reading sources... [100%] index                                                                                                                                                                                                               
/usr/local/google/home/romann/anaconda3/lib/python3.9/dataclasses.py:docstring of dataclasses.asdict:8: WARNING: Unexpected indentation.
/usr/local/google/home/romann/anaconda3/lib/python3.9/dataclasses.py:docstring of dataclasses.make_dataclass:14: WARNING: Unexpected indentation.
/usr/local/google/home/romann/anaconda3/lib/python3.9/dataclasses.py:docstring of dataclasses.replace:7: WARNING: Unexpected indentation.
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [100%] index                                                                                                                                                                                                                
generating indices... genindex py-modindex done
writing additional pages... search done
copying static files... done
copying extra files... done
dumping search index in English (code: en)... done
dumping object inventory... done
build succeeded, 3 warnings.

The HTML pages are in _build.

And the _build/index.html will render some code blocks as regular text. After the changes there are no warnings, and the rendering works correctly, let me know if this works!

@ericvsmith ericvsmith self-assigned this Jun 1, 2022
@ericvsmith
Copy link
Member

ericvsmith commented Jul 26, 2022

It turns out I'm too busy or lazy to test this with Sphinx, but the changes don't look unreasonable and can always be reverted.

GH is complaining about tests not passing, so I'm going to close then re-open this. I'm not sure if we'll need to update the PR branch or not, we'll see.

@ericvsmith ericvsmith closed this Jul 26, 2022
@ericvsmith ericvsmith reopened this Jul 26, 2022
@ericvsmith ericvsmith added the needs backport to 3.11 only security fixes label Jul 26, 2022
@ericvsmith ericvsmith merged commit b8c5286 into python:main Jul 26, 2022
@miss-islington
Copy link
Contributor

Thanks @romanngg for the PR, and @ericvsmith for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-95286 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jul 26, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 26, 2022
Previously, when using `functools.wrap` around them (and inherit their docstrings), sphinx renders the docstrings badly and raises warnings about wrong indent.
(cherry picked from commit b8c5286)

Co-authored-by: Roman Novak <[email protected]>
ambv pushed a commit that referenced this pull request Jul 27, 2022
Previously, when using `functools.wrap` around them (and inherit their docstrings), sphinx renders the docstrings badly and raises warnings about wrong indent.
(cherry picked from commit b8c5286)

Co-authored-by: Roman Novak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants