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-76846, GH-85281: Call __new__() and __init__() on pathlib subclasses #102789

Merged
merged 9 commits into from
Apr 3, 2023

Conversation

barneygale
Copy link
Contributor

@barneygale barneygale commented Mar 17, 2023

This PR fixes an issue where __new__() and __init__() were not called on subclasses of pathlib.PurePath and Path in some circumstances.

Specifically, the _from_parsed_parts() constructor -- which is used when iterating directories, parents, and in with_name() and friends -- has been altered as follows:

diff --git a/Lib/pathlib.py b/Lib/pathlib.py
index 55c44f12e5..c9b45ddbfa 100644
--- a/Lib/pathlib.py
+++ b/Lib/pathlib.py
@@ -310,7 +310,9 @@ def _from_parts(cls, args):
 
     @classmethod
     def _from_parsed_parts(cls, drv, root, parts):
-        self = object.__new__(cls)
+        path = cls._format_parsed_parts(drv, root, parts)
+        self = cls(path)
+        self._str = path or '.'
         self._drv = drv
         self._root = root
         self._parts = parts

This change alone has an unfortunate effect: paths constructed this way are re-parsed and re-normalized even though we have the fully normalized path at hand.

To fix this, we change the main constructor to not normalize paths. Instead, paths are normalized on-demand. This also speeds up path construction, p.joinpath(q), and p / q.

Copy link
Member

@zooba zooba left a comment

Choose a reason for hiding this comment

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

LGTM (with what I marked). Any concerns in particular that you'd like me to take a closer look at?

Lib/pathlib.py Outdated Show resolved Hide resolved
Lib/pathlib.py Outdated Show resolved Hide resolved
Lib/pathlib.py Outdated Show resolved Hide resolved
Lib/pathlib.py Outdated Show resolved Hide resolved
Co-authored-by: Steve Dower <steve.dower@microsoft.com>
Lib/pathlib.py Outdated Show resolved Hide resolved
Lib/test/test_pathlib.py Outdated Show resolved Hide resolved
@barneygale
Copy link
Contributor Author

I think those were my only areas of concern! Thanks for the review :)

@barneygale
Copy link
Contributor Author

Are you happy for me to merge, @zooba?

@zooba
Copy link
Member

zooba commented Apr 3, 2023

All yours :shipit:

@barneygale barneygale merged commit 11c3020 into python:main Apr 3, 2023
gaogaotiantian pushed a commit to gaogaotiantian/cpython that referenced this pull request Apr 8, 2023
…pathlib subclasses (pythonGH-102789)

Fix an issue where `__new__()` and `__init__()` were not called on subclasses of `pathlib.PurePath` and `Path` in some circumstances.

Paths are now normalized on-demand. This speeds up path construction, `p.joinpath(q)`, and `p / q`.

Co-authored-by: Steve Dower <steve.dower@microsoft.com>
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
…pathlib subclasses (pythonGH-102789)

Fix an issue where `__new__()` and `__init__()` were not called on subclasses of `pathlib.PurePath` and `Path` in some circumstances.

Paths are now normalized on-demand. This speeds up path construction, `p.joinpath(q)`, and `p / q`.

Co-authored-by: Steve Dower <steve.dower@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants