-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
bpo-28516: contextlib.ExitStack.__enter__ has trivial but undocument… #31636
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
bpo-28516: contextlib.ExitStack.__enter__ has trivial but undocument… #31636
Conversation
…ed behavior The enter_context is updated with following information: 'The :meth:`__enter__` method returns the ExitStack instance, and performs no additional operations.'
Doc/library/contextlib.rst
Outdated
@@ -529,7 +529,8 @@ Functions and classes provided: | |||
|
|||
Enters a new context manager and adds its :meth:`__exit__` method to | |||
the callback stack. The return value is the result of the context | |||
manager's own :meth:`__enter__` method. | |||
manager's own :meth:`__enter__` method.The :meth:`__enter__` method |
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.
space after period.
Doc/library/contextlib.rst
Outdated
@@ -529,7 +529,8 @@ Functions and classes provided: | |||
|
|||
Enters a new context manager and adds its :meth:`__exit__` method to | |||
the callback stack. The return value is the result of the context | |||
manager's own :meth:`__enter__` method. | |||
manager's own :meth:`__enter__` method.The :meth:`__enter__` method | |||
returns the ExitStack instance, and performs no additional operations. |
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 we make a link for ExitStack?
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.
Sure :).
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 for your comments. It is updated in the latest commit.
Added space after period and link to ExitStack.
Doc/library/contextlib.rst
Outdated
@@ -529,7 +529,8 @@ Functions and classes provided: | |||
|
|||
Enters a new context manager and adds its :meth:`__exit__` method to | |||
the callback stack. The return value is the result of the context | |||
manager's own :meth:`__enter__` method. | |||
manager's own :meth:`__enter__` method. The :meth:`__enter__` method |
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 doesn't make sense to put this in the enter_context
documentation. It needs to go somewhere in the ExitStack intro, perhaps in the paragraph after the example.
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 put it as a separate paragraph in the ExitStack intro. Please review.
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.
Are you able to review the last commit? If you feel anything to be updated pls let me know.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Doc/library/contextlib.rst
Outdated
Each instance maintains a stack of registered callbacks that are called in | ||
reverse order when the instance is closed (either explicitly or implicitly | ||
at the end of a :keyword:`with` statement). Note that callbacks are *not* | ||
invoked implicitly when the context stack instance is garbage collected. | ||
|
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.
Please put this line back
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.
Added the line in the latest commit. please review.
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.
The CI build is still failing due to some trailing whitespace.
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 cannot see any trailing spaces. Are you able to locate which line it is?
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.
Doc/library/contextlib.rst
Outdated
@@ -502,6 +502,9 @@ Functions and classes provided: | |||
# the with statement, even if attempts to open files later | |||
# in the list raise an exception | |||
|
|||
The :meth:`__enter__` method returns the :class:`ExitStack` instance, and |
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.
The :meth:`__enter__` method returns the :class:`ExitStack` instance, and | |
The :meth:`__enter__` method returns the :class:`ExitStack` instance, and |
Removed trailing whitespace.
Thanks Alex. I removed the trailing whitespace. Is it possible for me to run the CI tests locally? |
Thanks @Vidhyavinu for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10. |
GH-32145 is a backport of this pull request to the 3.9 branch. |
…-31636) The enter_context is updated with following information: 'The :meth:`__enter__` method returns the ExitStack instance, and performs no additional operations.' Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> (cherry picked from commit 86384cf) Co-authored-by: vidhya <96202776+Vidhyavinu@users.noreply.github.com>
…-31636) The enter_context is updated with following information: 'The :meth:`__enter__` method returns the ExitStack instance, and performs no additional operations.' Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> (cherry picked from commit 86384cf) Co-authored-by: vidhya <96202776+Vidhyavinu@users.noreply.github.com>
Not sure what happened to the 3.10 backport |
Looks like it's here: Idk why Bedevere didn't pick up on it. |
Yes, there's a section in the devguide with instructions on it here 🙂 |
Thanks @Vidhyavinu for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10. |
Thanks @Vidhyavinu for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10. |
GH-32145) The enter_context is updated with following information: 'The :meth:`__enter__` method returns the ExitStack instance, and performs no additional operations.' Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> (cherry picked from commit 86384cf) Co-authored-by: vidhya <96202776+Vidhyavinu@users.noreply.github.com> Co-authored-by: Ned Deily <nad@python.org>
@JelleZijlstra The 3.10 backport still seems to be missing in action. Sorry about that. If you don't mind, I'll let you deal with it as I've caused enough trouble already ;) |
Sure, I'll take care of it. Thanks! |
GH-32171 is a backport of this pull request to the 3.10 branch. |
…ythonGH-31636) The enter_context is updated with following information: 'The :meth:`__enter__` method returns the ExitStack instance, and performs no additional operations.' Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> (cherry picked from commit 86384cf) Co-authored-by: vidhya <96202776+Vidhyavinu@users.noreply.github.com>
…H-31636) (GH-32171) The enter_context is updated with following information: 'The :meth:`__enter__` method returns the ExitStack instance, and performs no additional operations.' Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> (cherry picked from commit 86384cf) Co-authored-by: vidhya <96202776+Vidhyavinu@users.noreply.github.com>
…-31636) (pythonGH-32145) The enter_context is updated with following information: 'The :meth:`__enter__` method returns the ExitStack instance, and performs no additional operations.' Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> (cherry picked from commit 86384cf) Co-authored-by: vidhya <96202776+Vidhyavinu@users.noreply.github.com> Co-authored-by: Ned Deily <nad@python.org>
The enter_context is updated with following information: 'The :meth:
__enter__
methodreturns the ExitStack instance, and performs no additional operations.'
https://bugs.python.org/issue28516