-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-100989: Revert #100990: Improve the accuracy of collections.deque docstrings #102949
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
gh-100989: Revert #100990: Improve the accuracy of collections.deque docstrings #102949
Conversation
timobrembeck
commented
Mar 23, 2023
- Issue: Accommodate Sphinx option by changing docstring return type of "integer" to "int". #100989
I verify that this PR fixes the IDLE tooltip regression of #100990. |
e4ae5a1
to
317f39b
Compare
Please do not force-push; instead do |
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; let's wait for Raymond to chime in.
The original commits should be reverted entirely. There is no known user problem that warrants a wholesale rewrite of the docstrings. In addition, the existing docstrings were modeled after their counterparts in
I've taught Python to new people weekly for 11 years. I've seen no evidence at all that these docstrings cause any issues for beginners. |
This reverts commit e5566e5.
…deque" This reverts commit 317f39b.
…cstrings (python#100990)" This reverts commit c740736.
@rhettinger Alright, I appreciate the insight! The motivation for this was not that I think the current docstrings are unclear in any way, but simply that I think my changes provide a compromise that improves compatibility with existing conventions and tools while not degrading understandability for humans at the same time. I mean of course I could also adapt the docstrings of their counterparts if this is the main concern, but as I understand your comments, this is more of a matter of principle, and probably other problems will arise if we go down that road. |
In the absence of a known user problem with readability, we should leave the docstrings in sync with |
…que" This reverts commit 269fe64.
@erlend-aasland Are you on-board with closing this PR and reverting the others? The original PR did not follow my review comments. It violated our docstring pep resulting which affected tooling and degraded the user experience. It broke the intentional symmetry with If there are new docstring conventions that we want to require, they should be proposed as an amendment to the docstring PEP rather than being pushed through under the guise of "fixing accuracy". |
Ok, I reverted all changes but this one. When this PR is squash merged, it will revert the old PR and include only the change from integer to int. |
The existing docs mention PEP-7 which says:
So a first step would be to update the docs and state that PEP-7 is no longer the preferred format. |
Also, PEP-257, which was mentioned in the discussion, says:
And especially the last paragraph sounds to me as this format would in fact be appropriate for C functions. |
@rhettinger: sure, I'm fine with reverting #100990, and AFAICS, this PR has now been updated to do just that (except for the integer => int thing). |
Thanks. In the interest of making sure it is done correctly and not overlooking any spurious changes, I'm going to do clean 100% revert of the previous PRs. A new clean PR can be made for just the "integer" to "int" change and applied only to 3.12 and 3.11. |
SGTM |