classification
Title: pathlib.Path objects can be used as context managers
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Isaac Muse, barneygale, brett.cannon, pitrou
Priority: normal Keywords: patch

Created on 2020-02-19 02:10 by barneygale, last changed 2020-04-01 14:20 by Antony.Lee. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 18846 merged barneygale, 2020-03-08 10:09
Messages (14)
msg362244 - (view) Author: Barney Gale (barneygale) * Date: 2020-02-19 02:10
`pathlib.Path` objects can be used as context managers, but this functionality is undocumented and makes little sense. Example:

>>> import pathlib
>>> root = pathlib.Path("/")
>>> with root:
...     print(1)
... 
1
>>> with root:
...     print(2)
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/barney/.pyenv/versions/3.7.3/lib/python3.7/pathlib.py", line 1028, in __enter__
    self._raise_closed()
  File "/home/barney/.pyenv/versions/3.7.3/lib/python3.7/pathlib.py", line 1035, in _raise_closed
    raise ValueError("I/O operation on closed path")
ValueError: I/O operation on closed path

`Path` objects don't acquire any resources on __new__/__init__/__enter__, nor do they release any resources on __exit__. The whole notion of the path being `_closed` seems to exist purely to make impure `Path` methods unusable after exiting from the context manager. I can't personally think of a compelling use case for this, and suggest that it be removed.
msg362840 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2020-02-27 21:17
A use-case of "closing" a path is to cheaply flag that a path object is no longer valid, e.g. you deleted the underlying file and so you don't want others using the object anymore without paying the penalty of having to do the I/O to trigger the failure due to the path no longer existing.

Now whether anyone is using this functionality I don't know, but removing it will inevitably break code for someone, so someone will need to make at least some form of an attempt to see if there's any use in the wild to justify simplifying the code by removing the functionality.
msg362934 - (view) Author: Antony Lee (Antony.Lee) * Date: 2020-02-28 22:53
A problem of having this bit of state in paths is that it violates assumptions based on immutability/hashability, e.g. with

    from pathlib import Path

    p = Path("foo")
    q = Path("foo")

    with p: pass

    unique_paths = {p, q}

    print(len(unique_paths))  # == 1, as p == q
    for path in unique_paths:
        print(path.resolve())  # Fails if the path is closed.

The last line fails with `unique_paths = {p, q}` as p has been closed (and apparently sets keep the first element when multiple "equal" elements are passed in), but not with `unique_paths = {q, p}`.

It would also prevent optimizations based on immutability as proposed in https://bugs.python.org/issue39783.
msg363206 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2020-03-02 19:59
I guess a question is whether we want immutability guarantees (I for one didn't even know you could hash Path objects until now).

I'm personally fine with that idea as it mentally makes sense to not need paths to be mutable. But as I said, someone needs to take at least some stab at seeing if any preexisting code out there will break if the _closed flag is removed before we can continue this discussion.
msg363209 - (view) Author: Antony Lee (Antony.Lee) * Date: 2020-03-02 20:30
Immutability and hashability are listed first among "general properties" of paths (https://docs.python.org/3/library/pathlib.html#general-properties), and in the PEP proposing pathlib (https://www.python.org/dev/peps/pep-0428/#immutability).  Looking at it again I *guess* the docs version could be read as only guaranteeing this for PurePaths (because that's in the PurePath section) rather than Path, but the PEP version clearly implies that this is true for both PurePaths and concrete Paths.

It may be tricky to check usage in third-party packages, given that one would need to look for `with <path-object>: ...` rather than, say, a method name which can be grepped.  Do you have any suggestions here?
msg363218 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2020-03-02 23:52
As with the Accessor abstraction, the original idea was to support Path objects backed by a directory file descriptor (for use with openat() and friends).  That idea was abandoned but it looks like the context manager stayed.  It's certainly not useful currently.
msg363260 - (view) Author: Barney Gale (barneygale) * Date: 2020-03-03 14:35
Can I ask what sort of backwards-compatibility guarantees Python provides for these cases? In the case of using a Path object as a context manager, I think we can say:

- It's easy to do - there's no need to call any underscore-prefixed methods for example
- It's undocumented
- It's pretty hard to determine existing usage statically - grepping for `with p:` is years of work.
msg363261 - (view) Author: Barney Gale (barneygale) * Date: 2020-03-03 14:36
Also, thank you Antoine for your explanation :-)
msg363285 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2020-03-03 18:08
> Can I ask what sort of backwards-compatibility guarantees Python provides for these cases?

A deprecation warning for two releases (i.e. two years). Then it can be removed. So if people want to move forward with removing this then a DeprecationWarning would need to be raised in all places where _closed is set with a message stating the functionality is deprecated in 3.9 and slated for removal in 3.11 (along with appropriate doc updates, both for the module and What's New).
msg363298 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2020-03-03 20:34
Note you could simply remove the "closed" flag and the context manager live. That way, you don't have to deprecate anything.
msg363383 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2020-03-04 19:30
@Antoine I just quite follow what you mean. Are you saying ditch _closed and just leave the context manager to be a no-op?
msg363468 - (view) Author: Isaac Muse (Isaac Muse) Date: 2020-03-06 01:46
Brace expansion does not currently exist in Python's glob. You'd have to use a third party module to expand the braces and then run glob on each returned pattern, or use a third party module that implements a glob that does it for you.

Shameless plug:

Brace expansion: https://github.com/facelessuser/bracex

Glob that does it for you (when the feature is enabled): https://github.com/facelessuser/wcmatch

Now whether Python should integrate such behavior by default is another question.
msg363469 - (view) Author: Isaac Muse (Isaac Muse) Date: 2020-03-06 01:46
Wrong thread sorry
msg365473 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2020-04-01 14:11
New changeset 00002e6d8b0ccdb6e0d9e98a9a7f9c9edfdf1311 by Barney Gale in branch 'master':
bpo-39682: make `pathlib.Path` immutable by removing (undocumented) support for "closing" a path by using it as a context manager (GH-18846)
https://github.com/python/cpython/commit/00002e6d8b0ccdb6e0d9e98a9a7f9c9edfdf1311
History
Date User Action Args
2020-04-01 14:20:19Antony.Leesetnosy: - Antony.Lee
2020-04-01 14:11:12pitrousetversions: + Python 3.9, - Python 3.8
2020-04-01 14:11:09pitrousetstatus: open -> closed
type: behavior
resolution: fixed
stage: patch review -> resolved
2020-04-01 14:11:00pitrousetmessages: + msg365473
2020-03-08 10:09:21barneygalesetkeywords: + patch
stage: patch review
pull_requests: + pull_request18203
2020-03-06 01:46:48Isaac Musesetmessages: + msg363469
2020-03-06 01:46:13Isaac Musesetnosy: + Isaac Muse
messages: + msg363468
2020-03-04 19:30:06brett.cannonsetmessages: + msg363383
2020-03-03 20:34:17pitrousetmessages: + msg363298
2020-03-03 18:08:24brett.cannonsetmessages: + msg363285
2020-03-03 14:36:15barneygalesetmessages: + msg363261
2020-03-03 14:35:33barneygalesetmessages: + msg363260
2020-03-02 23:52:49pitrousetmessages: + msg363218
2020-03-02 20:30:05Antony.Leesetmessages: + msg363209
2020-03-02 19:59:34brett.cannonsetmessages: + msg363206
2020-02-28 22:53:43Antony.Leesetnosy: + Antony.Lee
messages: + msg362934
2020-02-27 21:17:26brett.cannonsetmessages: + msg362840
2020-02-19 03:50:24xtreaksetnosy: + brett.cannon, pitrou
2020-02-19 02:10:35barneygalecreate