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

gh-110893: Improve docs for __future__ module #112348

Closed
wants to merge 8 commits into from

Conversation

anujrmohite
Copy link

@anujrmohite anujrmohite commented Nov 23, 2023

Related Issue

#110893 Improvements in the documentation future module

Changed file:

Doc/library/future.rst


📚 Documentation preview 📚: https://cpython-previews--112348.org.readthedocs.build/en/112348/library/__future__.html

Copy link

cpython-cla-bot bot commented Nov 23, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app bedevere-app bot added awaiting review docs Documentation in the Doc dir skip news labels Nov 23, 2023
@AlexWaygood AlexWaygood changed the title Added Improvements for future module #110893 gh-110893: Improve docs for __future__ module Nov 23, 2023
@anujrmohite
Copy link
Author

anujrmohite commented Nov 23, 2023

hey @merwok @nedbat ,
I've submitted a PR... . Could you please take a look? Your feedback and review are highly appreciated.
Thanks!

Doc/library/__future__.rst Outdated Show resolved Hide resolved
Doc/library/__future__.rst Outdated Show resolved Hide resolved
Doc/library/__future__.rst Outdated Show resolved Hide resolved
Doc/library/__future__.rst Outdated Show resolved Hide resolved
Doc/library/__future__.rst Outdated Show resolved Hide resolved
Doc/library/__future__.rst Outdated Show resolved Hide resolved
@anujrmohite
Copy link
Author

anujrmohite commented Dec 1, 2023

hey @merwok @hugovk, I've finished the changes. Please review at your convenience.
Happy to make any further adjustments if needed.

Copy link
Contributor

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR and for contributing to CPython!

However, I think this still isn't clear enough about the ways in which __future__ is special, and the ways in which it is normal. I think this also regresses clarity in a few ways, e.g. talking about OptionalRelease and MandatoryRelease before these terms are defined, losing the fact that MandatoryRelease is only a prediction and not a guarantee, etc. I merged https://github.com/python/cpython/pull/114642/files which attempts to address the first point here.

Deleting initial spaces was a mistake.  PEP 8 comments are sentences.
@terryjreedy
Copy link
Member

A separate PR has made an alternate replacement of the initial 3 bullet points. Both replacements need to be read and merged into one expanded replacement.

@nineteendo
Copy link
Contributor

Could someone solve the merge conflicts?

@terryjreedy
Copy link
Member

@JelleZijlstra Could you fix the merge conflicts between your PR and the first 3 paragraphs of this PR? I don't care or have any opinion how. If you think your version completely supercedes what it conflicts with, and no merging is needed, it would be trivial.

@JelleZijlstra
Copy link
Member

I fixed the merge conflict between @hauntsaninja's PR that I merged some time ago and this one. I picked the text that we already have for the introductory paragraphs.

The remaining changes in this PR don't solve any real problem as far as I can see, and I generally prefer the current wording over the changes in this PR. So I suggest closing this PR.

With the merge resolution I pushed, this PR no longer removes the bullet points at the top of the __future__ documentation that justify the design. Possibly those should be removed or moved, as they're not that relevant to current usage.

@merwok
Copy link
Member

merwok commented May 19, 2024

I agree with Jelle!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants