bpo-39682: make `pathlib.Path` immutable by removing (undocumented) support for "closing" a path by using it as a context manager #18846

barneygale · 2020-03-08T10:09:20Z

This implements @pitrou's suggestion of retaining support for using a Path object as a context manager, but making it a no-op.

https://bugs.python.org/issue39682

the-knights-who-say-ni · 2020-03-08T10:09:23Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@barneygale

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

pitrou

+1 on the principle, but I don't think we should remove the test entirely, just change it.
Also, please add a NEWS entry. You can do so using blurb or blurb-it.

Lib/test/test_pathlib.py

bedevere-bot · 2020-03-08T10:31:06Z

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

barneygale · 2020-03-08T11:02:33Z

I have made the requested changes; please review again. Thank you!

bedevere-bot · 2020-03-08T11:02:36Z

Thanks for making the requested changes!

@pitrou: please review the changes made to this pull request.

pitrou

+1. Have you signed the CLA?

barneygale · 2020-03-08T11:19:51Z

I have! I think it's just making its way through the system currently.

pitrou · 2020-03-08T11:21:32Z

Cool, thank you. I'll wait for the bot to notice, then :-)

brettcannon

Quick request to document in the code why this change is being made, else someone else in the future may be wondering why it's a no-op.

Lib/test/test_pathlib.py

Lib/pathlib.py

Misc/NEWS.d/next/Library/2020-03-08-11-00-01.bpo-39682.AxXZNz.rst

bedevere-bot · 2020-03-09T21:57:33Z

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

barneygale · 2020-03-10T00:07:12Z

Thank you Brett. I have made the requested changes; please review again. Hope the comment isn't too long.

bedevere-bot · 2020-03-10T00:07:16Z

Thanks for making the requested changes!

@brettcannon, @pitrou: please review the changes made to this pull request.

bedevere-bot · 2020-03-10T19:05:43Z

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

barneygale · 2020-03-21T01:51:06Z

I have made the requested changes; please review again.

bedevere-bot · 2020-03-21T01:51:10Z

Thanks for making the requested changes!

@pitrou, @brettcannon: please review the changes made to this pull request.

Lib/pathlib.py

pitrou

+1 for this, thank you @barneygale .
I'll leave this open a day or two in case people hold strong opinions about deprecating this silent behaviour.

pitrou · 2020-03-28T09:39:18Z

@barneygale Can you rebase/merge from master to re-run CI on this?

…upport for "closing" a path by using it as a context manager. Support for using a path as a context manager remains, and is now a no-op. Co-Authored-By: Brett Cannon <54418+brettcannon@users.noreply.github.com>

barneygale · 2020-03-29T20:16:44Z

Rebased on master. Thanks @pitrou and @brettcannon for helping get this through :)

Misc/NEWS.d/next/Library/2020-03-08-11-00-01.bpo-39682.AxXZNz.rst

Co-Authored-By: Kyle Stanley <aeros167@gmail.com>

aeros

Thanks @barneygale; LGTM.

brettcannon · 2020-03-30T20:22:23Z

I have an opinion about deprecation, but it isn't strong. 😄 Basically I'm worried someone out there is relying on this functionality and is going to be caught off-guard that it suddenly stopped working. But as I said, I'm +0 on the deprecation so I won't hold up the PR if people disagree.

barneygale · 2020-04-03T17:50:44Z

Just noting that I have a follow-up PR - #19342 - that removes the _init() method that was touched by this PR.

the-knights-who-say-ni added the CLA not signed label Mar 8, 2020

bedevere-bot added the awaiting review label Mar 8, 2020

pitrou requested changes Mar 8, 2020

View reviewed changes

Lib/test/test_pathlib.py Outdated Show resolved Hide resolved

bedevere-bot removed the awaiting review label Mar 8, 2020

bedevere-bot added the awaiting changes label Mar 8, 2020

barneygale force-pushed the bpo-39682-pathlib-path-context-manager branch from 626c970 to c89a62d Compare March 8, 2020 10:51

bedevere-bot added awaiting change review and removed awaiting changes labels Mar 8, 2020

bedevere-bot requested a review from pitrou March 8, 2020 11:02

pitrou approved these changes Mar 8, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Mar 8, 2020

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Mar 9, 2020

brettcannon requested changes Mar 9, 2020

View reviewed changes

Lib/test/test_pathlib.py Outdated Show resolved Hide resolved

Lib/pathlib.py Show resolved Hide resolved

Misc/NEWS.d/next/Library/2020-03-08-11-00-01.bpo-39682.AxXZNz.rst Outdated Show resolved Hide resolved

bedevere-bot removed the awaiting merge label Mar 9, 2020

bedevere-bot added the awaiting changes label Mar 9, 2020

barneygale force-pushed the bpo-39682-pathlib-path-context-manager branch from 19bea17 to b260194 Compare March 10, 2020 00:05

bedevere-bot added awaiting change review and removed awaiting changes labels Mar 10, 2020

bedevere-bot requested a review from brettcannon March 10, 2020 00:07

bedevere-bot added awaiting changes and removed awaiting change review labels Mar 10, 2020

barneygale force-pushed the bpo-39682-pathlib-path-context-manager branch from 86cf4dc to ca3acfd Compare March 21, 2020 01:50

bedevere-bot added awaiting change review and removed awaiting changes labels Mar 21, 2020

bedevere-bot requested a review from brettcannon March 21, 2020 01:51

barneygale commented Mar 21, 2020

View reviewed changes

Lib/pathlib.py Show resolved Hide resolved

pitrou approved these changes Mar 28, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Mar 28, 2020

bpo-39682: make pathlib.Path immutable by removing (undocumented) s…

31c116a

…upport for "closing" a path by using it as a context manager. Support for using a path as a context manager remains, and is now a no-op. Co-Authored-By: Brett Cannon <54418+brettcannon@users.noreply.github.com>

barneygale force-pushed the bpo-39682-pathlib-path-context-manager branch from ca3acfd to 31c116a Compare March 29, 2020 19:34

aeros reviewed Mar 30, 2020

View reviewed changes

Misc/NEWS.d/next/Library/2020-03-08-11-00-01.bpo-39682.AxXZNz.rst Outdated Show resolved Hide resolved

Update Misc/NEWS.d/next/Library/2020-03-08-11-00-01.bpo-39682.AxXZNz.rst

d9aa9df

Co-Authored-By: Kyle Stanley <aeros167@gmail.com>

aeros approved these changes Mar 30, 2020

View reviewed changes

pitrou merged commit 00002e6 into python:master Apr 1, 2020

bedevere-bot removed the awaiting merge label Apr 1, 2020

barneygale mentioned this pull request Apr 3, 2020

bpo-40038: pathlib: remove partial support for preserving accessor when modifying a path #19342

Merged

barneygale mannequin mentioned this pull request Apr 10, 2022

pathlib.Path objects can be used as context managers #83863

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-39682: make `pathlib.Path` immutable by removing (undocumented) support for "closing" a path by using it as a context manager #18846

bpo-39682: make `pathlib.Path` immutable by removing (undocumented) support for "closing" a path by using it as a context manager #18846

barneygale commented Mar 8, 2020 •

edited by bedevere-bot

Loading

the-knights-who-say-ni commented Mar 8, 2020

pitrou left a comment

bedevere-bot commented Mar 8, 2020

barneygale commented Mar 8, 2020

bedevere-bot commented Mar 8, 2020

pitrou left a comment

barneygale commented Mar 8, 2020

pitrou commented Mar 8, 2020

brettcannon left a comment

bedevere-bot commented Mar 9, 2020

barneygale commented Mar 10, 2020

bedevere-bot commented Mar 10, 2020

bedevere-bot commented Mar 10, 2020

barneygale commented Mar 21, 2020

bedevere-bot commented Mar 21, 2020

pitrou left a comment

pitrou commented Mar 28, 2020

barneygale commented Mar 29, 2020

aeros left a comment

brettcannon commented Mar 30, 2020 •

edited

Loading

barneygale commented Apr 3, 2020 •

edited

Loading

bpo-39682: make pathlib.Path immutable by removing (undocumented) support for "closing" a path by using it as a context manager #18846

bpo-39682: make pathlib.Path immutable by removing (undocumented) support for "closing" a path by using it as a context manager #18846

Conversation

barneygale commented Mar 8, 2020 • edited by bedevere-bot Loading

the-knights-who-say-ni commented Mar 8, 2020

CLA Missing

pitrou left a comment

Choose a reason for hiding this comment

bedevere-bot commented Mar 8, 2020

barneygale commented Mar 8, 2020

bedevere-bot commented Mar 8, 2020

pitrou left a comment

Choose a reason for hiding this comment

barneygale commented Mar 8, 2020

pitrou commented Mar 8, 2020

brettcannon left a comment

Choose a reason for hiding this comment

bedevere-bot commented Mar 9, 2020

barneygale commented Mar 10, 2020

bedevere-bot commented Mar 10, 2020

bedevere-bot commented Mar 10, 2020

barneygale commented Mar 21, 2020

bedevere-bot commented Mar 21, 2020

pitrou left a comment

Choose a reason for hiding this comment

pitrou commented Mar 28, 2020

barneygale commented Mar 29, 2020

aeros left a comment

Choose a reason for hiding this comment

brettcannon commented Mar 30, 2020 • edited Loading

barneygale commented Apr 3, 2020 • edited Loading

bpo-39682: make `pathlib.Path` immutable by removing (undocumented) support for "closing" a path by using it as a context manager #18846

bpo-39682: make `pathlib.Path` immutable by removing (undocumented) support for "closing" a path by using it as a context manager #18846

barneygale commented Mar 8, 2020 •

edited by bedevere-bot

Loading

brettcannon commented Mar 30, 2020 •

edited

Loading

barneygale commented Apr 3, 2020 •

edited

Loading