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

pathlib.Path._from_parsed_parts should call cls.__new__(cls) #76846

Closed
qb-cea mannequin opened this issue Jan 25, 2018 · 7 comments
Closed

pathlib.Path._from_parsed_parts should call cls.__new__(cls) #76846

qb-cea mannequin opened this issue Jan 25, 2018 · 7 comments
Labels
stdlib Python modules in the Lib dir topic-pathlib type-bug An unexpected behavior, bug, or error

Comments

@qb-cea
Copy link
Mannequin

qb-cea mannequin commented Jan 25, 2018

BPO 32665
Nosy @pitrou, @pablogsal, @qb-cea
Files
  • reproducer.py: reproducer (tested with python3.4)
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2018-01-25.12:27:25.104>
    labels = ['type-bug', 'library']
    title = 'pathlib.Path._from_parsed_parts should call cls.__new__(cls)'
    updated_at = <Date 2018-01-27.21:38:03.770>
    user = 'https://github.com/qb-cea'

    bugs.python.org fields:

    activity = <Date 2018-01-27.21:38:03.770>
    actor = 'pablogsal'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2018-01-25.12:27:25.104>
    creator = 'qb-cea'
    dependencies = []
    files = ['47409']
    hgrepos = []
    issue_num = 32665
    keywords = []
    message_count = 5.0
    messages = ['310671', '310758', '310888', '310889', '310890']
    nosy_count = 3.0
    nosy_names = ['pitrou', 'pablogsal', 'qb-cea']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = None
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue32665'
    versions = ['Python 3.6']

    Linked PRs

    @qb-cea
    Copy link
    Mannequin Author

    qb-cea mannequin commented Jan 25, 2018

    Hi,

    I tried subclassing pathlib.Path and provide it with a new attribute (basically an accessor to an extended attribute). I am rather new to the concept of __slots__ and __new__() but here is how I pictured it should look:

    from errno import ENODATA
    from os import getxattr, setxattr
    from pathlib import Path
    
    class Path_(type(Path())):
    
        __slots__ = ("new_attr",)
    
        def __new__(cls, *args, new_attr=None, **kwargs):
            self = super().__new__(cls, *args, **kwargs)
            self._new_attr = new_attr
            return self
    
        @property
        def new_attr(self):
            if self._new_attr:
                return self._new_attr
        try:
            new_attr = getxattr(self, "user.new_attr")
        except OSError as exc:
            if exc.errno != ENODATA:
                raise exc
        else:
            self._new_attr = new_attr
            return new_attr
    
            new_attr = b"something_dynamic" # for example uuid4().bytes
            setxattr(self, "user.new_attr", new_attr)
            self._new_attr = new_attr
            return new_attr

    The issue I have is that although my class defines its own __new__() method, it is not always called by the methods of pathlib.Path. For example:

    >>> Path_("/etc").parent.new_attr
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/path/to/reproducer.py", line 19, in new_attr
        if self._new_attr:
    AttributeError: _new_attr

    The current workaround I use consists in redefining pathlib.Path's _from_parsed_parts() method in my class: instead of creating a new object using:
    object.__new__(cls)
    my implementation uses:
    cls.__new__(cls)

    This is the first time I play with the __new__() special method, so it is possible I missed something, if so, sorry for the noise.

    @qb-cea qb-cea mannequin added the stdlib Python modules in the Lib dir label Jan 25, 2018
    @qb-cea
    Copy link
    Mannequin Author

    qb-cea mannequin commented Jan 26, 2018

    Typo in the code of the previous comment:

    • __slots__ = ("new_attr",)
      + __slots__ = ("_new_attr",)

    @qb-cea qb-cea mannequin added the type-bug An unexpected behavior, bug, or error label Jan 26, 2018
    @pablogsal
    Copy link
    Member

    This behaviour is because "parent" descriptor ends calling:

        @classmethod
        def _from_parsed_parts(cls, drv, root, parts, init=True):
            self = object.__new__(cls)
            self._drv = drv
            self._root = root
            self._parts = parts
            if init:
                self._init()
            return self

    and this calls object.__new__ and this call raises AttributeError: new_attr. Notice that object.__new__(cls) will not raise as this snippet shows:

    >>>: class A:
    ...: def __new__(*args):
    ...: raise ZeroDivisionError()
    ...:

    >> A()
    ---------------------------------------------------------------------------

    ZeroDivisionError                         Traceback (most recent call last)
    <python> in <module>()
    ----> 1 A()

    <python> in __new__(*args)
    1 class A:
    2 def __new__(*args):
    ----> 3 raise ZeroDivisionError()
    4

    ZeroDivisionError:

    >> object.__new__(A)
    >> <main.A at 0x7f6239c17860>

    @pablogsal
    Copy link
    Member

    Sorry, the exception of object.__new__(cls) is AttributeError: _drv

    @pablogsal
    Copy link
    Member

    Antoine, is the reason of calling object.new(cls) in _from_parsed_parts that the code does not raise on creation?

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @barneygale
    Copy link
    Contributor

    barneygale commented Jan 5, 2023

    #100481 removes _from_parsed_parts() and adds a supported route for subclassing, so would resolve this issue I believe.

    barneygale added a commit to barneygale/cpython that referenced this issue Feb 4, 2023
    `PurePath` now normalises and splits paths only when necessary, e.g. when
    `.name` or `.parent` is accessed. The result is cached. This speeds up path
    object construction by around 4x.
    
    `PurePath.__fspath__()` now returns an unnormalised path, which should be
    transparent to filesystem APIs (else pathlib's normalisation is broken!).
    This extends the earlier performance improvement to most impure `Path`
    methods, and also speeds up pickling, `p.joinpath('bar')` and `p / 'bar'`.
    
    This also fixes pythonGH-76846 and pythonGH-85281 by unifying path constructors and
    adding an `__init__()` method.
    barneygale added a commit that referenced this issue Apr 3, 2023
    …lasses (GH-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>
    @barneygale
    Copy link
    Contributor

    Fixed in Python 3.12 / #102789 / 11c3020

    gaogaotiantian pushed a commit to gaogaotiantian/cpython that referenced this issue 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 issue 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
    Labels
    stdlib Python modules in the Lib dir topic-pathlib type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    Successfully merging a pull request may close this issue.

    3 participants