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

Path takes and ignores **kwargs #74033

Closed
JelleZijlstra opened this issue Mar 18, 2017 · 12 comments
Closed

Path takes and ignores **kwargs #74033

JelleZijlstra opened this issue Mar 18, 2017 · 12 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir topic-pathlib type-bug An unexpected behavior, bug, or error

Comments

@JelleZijlstra
Copy link
Member

BPO 29847
Nosy @brettcannon, @pitrou, @serhiy-storchaka, @jstasiak, @JelleZijlstra, @DimitrisJim, @remilapeyre, @uriyyo
PRs
  • bpo-29847: Path subclasses raise TypeError given kwargs #13399
  • gh-74033: Fix bug when Path takes and ignores **kwargs #19632
  • 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 2017-03-18.15:22:47.434>
    labels = ['type-bug', '3.8', '3.9', '3.10', '3.7', 'library']
    title = 'Path takes and ignores **kwargs'
    updated_at = <Date 2020-06-10.22:05:47.572>
    user = 'https://github.com/JelleZijlstra'

    bugs.python.org fields:

    activity = <Date 2020-06-10.22:05:47.572>
    actor = 'remi.lapeyre'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2017-03-18.15:22:47.434>
    creator = 'JelleZijlstra'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 29847
    keywords = ['patch']
    message_count = 9.0
    messages = ['289817', '289896', '289897', '289902', '289944', '289945', '289946', '289998', '369520']
    nosy_count = 8.0
    nosy_names = ['brett.cannon', 'pitrou', 'serhiy.storchaka', 'jstasiak', 'JelleZijlstra', 'Jim Fasarakis-Hilliard', 'remi.lapeyre', 'uriyyo']
    pr_nums = ['13399', '19632']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue29847'
    versions = ['Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10']

    @JelleZijlstra
    Copy link
    Member Author

    pathlib.Path.__new__ takes **kwargs, but doesn't do anything with them (https://github.com/python/cpython/blob/master/Lib/pathlib.py#L979). This doesn't appear to be documented.

    This feature should presumably be either documented or removed (probably removed unless I'm missing some reason for having it).

    Brief discussion on a typeshed PR at python/typeshed#991 (diff)

    @brettcannon
    Copy link
    Member

    Yep, kwargs should be dropped since it isn't used or documented: https://docs.python.org/3/library/pathlib.html#pathlib.PurePath (probably just a hold-over from when it did in some earlier version of the code).

    @brettcannon brettcannon added stdlib Python modules in the Lib dir 3.7 (EOL) end of life type-bug An unexpected behavior, bug, or error labels Mar 20, 2017
    @JelleZijlstra
    Copy link
    Member Author

    Thanks, I'll add a PR. This doesn't need to be documented, right?

    @serhiy-storchaka
    Copy link
    Member

    The support of **kwargs in Path.__new__ is needed if you want to implement a subclass of Path with __init__ accepting keyword arguments (and since Path constructor takes variable number of positional arguments, new arguments should be keyword-only).

    >>> import pathlib
    >>> class MyPath(pathlib.PosixPath):
    ...     def __init__(self, *args, spam=False):
    ...         self.spam = spam
    ... 
    >>> p = MyPath('/', spam=True)
    >>> p
    MyPath('/')
    >>> p.spam
    True

    Removing **kwargs from Path.__new__ will break the above example.

    >>> MyPath('/', spam=True)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: __new__() got an unexpected keyword argument 'spam'

    @brettcannon
    Copy link
    Member

    Shoot, that's too bad. I guess we should document it then so people are aware that keyword arguments are ignored, else we will break subclasses. There's also an unfortunate difference between PurePath and Path as PurePath doesn't have this quirk.

    @serhiy-storchaka
    Copy link
    Member

    I don't know whether it was the intension of Antoine or just an oversight. I don't know whether it is used in the wild. But we can at least raise a TypeError for concrete classes PosixPath and WindowsPath if ignoring keyword arguments is a problem. Many extension types don't take keyword arguments, but their subclasses accept and ignore keyword arguments. For example:

    >>> filter(None, [], foo=123)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: filter() does not take keyword arguments
    >>> class X(filter): pass
    ... 
    >>> X(None, [], foo=123)
    <__main__.X object at 0xb6fdcacc>

    @pitrou
    Copy link
    Member

    pitrou commented Mar 21, 2017

    The support of **kwargs in Path.__new__ is needed if you want to implement a subclass of Path with __init__ accepting keyword arguments

    I don't remember exactly, but I think this was the intention indeed. There was originally an openat-using subclass, and IIRC it took additional parameters (such as the directory fd). That got scrapped quite early in the process, so we can remove the **kwargs thing now.

    @brettcannon
    Copy link
    Member

    Then I vote for Serhiy's idea of simply raising an exception in the concrete subclasses when a keyword argument is given.

    @remilapeyre
    Copy link
    Mannequin

    remilapeyre mannequin commented May 21, 2020

    PurePath subclasses cannot support kwargs as __new__() does not accept **kwargs:

    >>> from pathlib import PurePath
    >>> class MyPurePath(PurePath):
    ...     def __init__(self, *args, **kargs): pass
    ... 
    >>> MyPurePath('foo', spam=True)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: __new__() got an unexpected keyword argument 'spam'

    The behaviour for this should probably be made the same for both Path and PurePath.

    @remilapeyre remilapeyre mannequin added 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes labels Jun 10, 2020
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @jaraco
    Copy link
    Member

    jaraco commented Oct 10, 2022

    As reported in the dupe bug, this bug led to masking a user mistake.

    @barneygale
    Copy link
    Contributor

    #100481 would fix this without breaking subclassing by adding an __init__() method.

    miss-islington pushed a commit that referenced this issue Jan 14, 2023
    Fix a bug where `Path` takes and ignores `**kwargs` by adding to `PurePath`  class `__init__` method which can take only positional arguments.
    
    Automerge-Triggered-By: GH:brettcannon
    @brettcannon
    Copy link
    Member

    I took #19632 as a fix, but it can obviously be tweaked as long as the deprecation warning sticks around.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir topic-pathlib type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants