Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-101100: Fix Sphinx warnings in turtle module#102340
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
hugovk commented Feb 28, 2023 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
CAM-Gerlach left a comment • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some top-level comments (some also reflected in individual suggestions):
- The
turtle.*callables are actually documented as module-level functions, rather than class-level methods on their specific class. ISTM that they should ideally be primarily documented as methods of their particular class, to be clear and explicit where each comes from and their recommended usage pattern, but I'm still working on a clean way to redirect these to avoid breaking existing references. So, at least for now the current definitions will have to do, but I would suggest at least using the correct role (funcrather thanmeth), and omitting theturtlewhich implies they are methods of aturtleclass (rather than their actual class. - Particularly in places where the actual class name is used, instead of erasing that information, I suggest instead making that the explicit title of the reference. Then, readers still have that info, and we can more easily go back later to have it point to the class if we end up moving and directing things.
- If you use
currentmodulewhere indicated, you can revert most of the noisy changes below that line.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
hugovk commented Mar 12, 2023
Thank you for the review! Remaining warnings: $ make -C Doc html SPHINXERRORHANDLING=-n 2>&1| grep turtle.rst | tee >(wc -l)/Users/huvankem/github/cpython/Doc/library/turtle.rst:58: WARNING: py:class reference target not found: tkinter.Canvas/Users/huvankem/github/cpython/Doc/library/turtle.rst:75: WARNING: py:class reference target not found: Pen/Users/huvankem/github/cpython/Doc/library/turtle.rst:1: WARNING: py:class reference target not found: tkinter.Canvas/Users/huvankem/github/cpython/Doc/library/turtle.rst:1: WARNING: py:class reference target not found: tkinter.Canvas 4 |
CAM-Gerlach left a comment • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I had some follow-up suggestions mostly related to the second point above,
Particularly in places where the actual class name is used, instead of erasing that information, I suggest instead making that the explicit title of the reference. Then, readers still have that info, and we can more easily go back later to have it point to the class if we end up moving and directing things.
In particular, a number of them didn't actually make sense anymore when the prose and reference text no longer referred to the class/method.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
CAM-Gerlach left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @hugovk !
miss-islington commented Mar 13, 2023
Thanks @hugovk for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11. |
miss-islington commented Mar 13, 2023
Sorry @hugovk, I had trouble checking out the |
hugovk commented Mar 13, 2023
Thank you for all the reviews! |
bedevere-bot commented Mar 13, 2023
GH-102638 is a backport of this pull request to the 3.10 branch. |
) (cherry picked from commit 78e4e6c) Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com> Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
miss-islington commented Mar 13, 2023
Thanks @hugovk for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11. |
) (cherry picked from commit 78e4e6c) Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com> Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
bedevere-bot commented Mar 13, 2023
GH-102639 is a backport of this pull request to the 3.11 branch. |
…nGH-102340) Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM> (cherry picked from commit 78e4e6c) Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Fixes 36 Sphinx warnings.
Before
After
I think the
tkinter.Canvasones are valid because I don't seeCanvasin https://github.com/python/cpython/blob/main/Doc/library/tkinter.rstAnd I'm not sure how to deal with
turtle.Pen, it's defined like:cpython/Lib/turtle.py
Line 3836 in 880437d
where:
cpython/Lib/turtle.py
Line 3816 in 880437d