Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-129667: Update annotation in documentation#129669
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
IIRC there was a decision to not litter the docs the trailing backslash. People find it confusing and it doesn't seem to help them in any way. |
donBarbos commentedFeb 5, 2025 • 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.
@rhettinger sorry, I fixed it. but other changes are relevant, isn't it? |
@rhettinger, the Editorial Board's recent decision was thatPEP-570 syntax should be used in documentation. This is now documented in the devguide; seepython/devguide#1344. The Editorial Board has a standing delegation from the Steering Council on all documentation matters:https://github.com/python/steering-council/blob/main/process/standing-delegations.md#documentation-editorial-board |
So should I put |
Then this PR can go forward. It's a bummer though. Signatures with trailing backslashes are intrinsically less readable. People who are only occasional users of Python find it confusing (likely because no other language does this). This makes the docs less accessible to average users. A high school student shouldn't have to see |
This reverts commitec2398f.
donBarbos commentedFeb 5, 2025 • 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.
I think they use other sources of knowledge, and most likely the docs reader is familiar with python syntax. And also it would be nice to add "skip news" label |
My direct experience says otherwise. When people take an introductory python course, the positional-only/keword-only syntax is rarely covered. There is just too much else to talk about. Introductory books on Python typically defer discussion on this topic until all other core topics have been covered. Also, it seems like an anti-goal to reduce accessiblity and expect some user categories to search elsewhere for knowledge. AFAICT, we've always wanted to maximize accessibility for all users. |
At a recent Editorial Board meeting, we re-affirmed the principle of the reference documentation being precise. The function signatures should have slashes to indicate arguments that are positional-only. It can be a bit confusing to new learners, so we'll be experimenting with adjustments to the Sphinx rendering to help people understand the syntax. |
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.
I'm a bit conflicted with the fact that we're now showing the internal parameters in the signatures. Yes, it could be helpful indeed, but it also exposes implementation details. For functions, I think we could implicitly not include them because an external caller is not meant to use them (probably), however for methods, if the class can be inherited, then children classes may want to know the complete signature just to be sure that they do not violate the Liskov substitution principle.
However, as I said on some functions/methods, I prefer that we don't add new parameters in this PR but only add the positional-only markers. In separate PRs, each added parameter should either explicitly be marked as internal (in the docs) or we should not show it if callers or subclasses do not need to know about it.
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.
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.
Can you first address the new comments and then check that all the remaining modifications are actually backportable to 3.12 as well? TiA
@@ -112,7 +112,7 @@ Interactive Interpreter Objects | |||
with it. | |||
.. method:: InteractiveInterpreter.showsyntaxerror(filename=None) | |||
.. method:: InteractiveInterpreter.showsyntaxerror(filename=None, **kwargs) |
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.
I don't really know whatkwargs means here so I would appreciate that it's documented in a separate PR as well.
@@ -194,6 +194,10 @@ And:: | |||
Default value of *max_workers* is changed to | |||
``min(32, (os.process_cpu_count() or 1) + 4)``. | |||
.. versionchanged:: next |
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.
It's not "next" as this was added in a previous release (just find the major.minor version where it was added, I think it's still 3.14)
Uh oh!
There was an error while loading.Please reload this page.
@@ -166,7 +166,7 @@ interpreter objects as well as the following additions. | |||
Print an exit message when exiting. | |||
.. method:: InteractiveConsole.push(line) | |||
.. method:: InteractiveConsole.push(line, filename=None, _symbol="single") |
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.
I think we should have a separate PR for re-documentingInteractiveConsole
methods. In addition, I'm still unsure whether private parameters should be exposed or not. Personally, I would rather prefer not having them exposed but if subclasses need to override the method, I think their signature should then be compatible. Note thathttps://github.com/python/typeshed/blob/1c17cd429c2f91b0066547deb99c537af3e54d39/stdlib/code.pyi#L23-L37 doesnot expose this private parameter, so I think we shouldn't as well (and let this inconsistency with the signature remain; mypy wouldn't complain as typeshed also doesn't specify it.
Finally,filename
seems to be only 3.13+ so we should also be careful with the backports (therefore I'd prefer having a separate PR forInteractiveConsole
)
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
📚 Documentation preview 📚:https://cpython-previews--129669.org.readthedocs.build/