Skip to content

bpo-29847: Path subclasses raise TypeError given kwargs #13399

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

Closed
wants to merge 443 commits into from
Closed

bpo-29847: Path subclasses raise TypeError given kwargs #13399

wants to merge 443 commits into from

Conversation

jsaporta
Copy link

@jsaporta jsaporta commented May 18, 2019

TypeError raised in the subclasses as per the discussion on bugs.python.org.

https://bugs.python.org/issue29847

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Our records indicate we have not received your CLA. For legal reasons we need you to sign this 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 your contribution, we look forward to reviewing it!

@jsaporta jsaporta changed the title bpo-29847: Path subclasses raise TypeError wit kwargs bpo-29847: Path subclasses raise TypeError given kwargs May 18, 2019
@@ -1512,13 +1512,23 @@ class PosixPath(Path, PurePosixPath):
"""
__slots__ = ()

def __new__(cls, *args, **kwargs):
if kwargs:
raise TypeError("keyword arguments provided but ignored")
Copy link
Contributor

Choose a reason for hiding this comment

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

What about start the raise with uppercase?

Copy link
Author

Choose a reason for hiding this comment

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

Other errors I've seen start with lowercase letters.

>>> 3 + 'string'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for +: 'int' and 'str'

Lib/pathlib.py Outdated
class WindowsPath(Path, PureWindowsPath):
"""Path subclass for Windows systems.

On a Windows system, instantiating a Path should return this object.
"""
__slots__ = ()

def __new__(cls, *args, **kwargs):
if kwargs:
Copy link
Contributor

Choose a reason for hiding this comment

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

This weird for me because if you don't have kwargs on __new__(cls, *args, **kwargs)
because this will raise a exception, what are you sending on super().__new__(cls, *args, **kwargs)

Copy link
Author

@jsaporta jsaporta May 18, 2019

Choose a reason for hiding this comment

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

Not sure I understand you clearly, but if kwargs is an empty dictionary, then it will act as a falsy value in the if statement, and the exception will not be raised.

@csabella csabella requested a review from pitrou May 18, 2019 11:34
@tirkarthi
Copy link
Member

I would suggest adding a NEWS entry and perhaps a versionchanged section to doc since someone could have been passing kwargs though it was silently ignored and undocumented in the past that now raises a TypeError.

@EpicWink
Copy link
Contributor

EpicWink commented May 20, 2019

What about a test to see if a new subclass of pathlib.Path (or pathlib.PosixPath or pathlib.WindowsPath) can still use keyword-only parameters in the initialiser?

import pathlib
class MyPath(type(pathlib.Path())):
    def __init__(self, *args, foo):
        self.foo = foo
path = MyPath('bar', foo=42)
assert path.foo == 42

jdemeyer and others added 22 commits May 30, 2019 12:43
…H-8096)

There is no need to clear these immutable objects during shutdown.
This reverts commit 71dc7c5. Turns out you must have write access for CODEOWNERS to work.
Bump the removal to 3.9, indicate collections.abc available since 3.3,
replace version-changed directive to deprecated-removed.


https://bugs.python.org/issue36953
…yping (GH-13685)

This is an old feature request that appears from time to time. After a year of experimenting with various introspection capabilities in `typing_inspect` on PyPI, I propose to add these two most commonly used functions: `get_origin()` and `get_args()`. These are essentially thin public wrappers around private APIs: `__origin__` and `__args__`.

As discussed in the issue and on the typing tracker, exposing some public helpers instead of `__origin__` and `__args__` directly will give us more flexibility if we will decide to update the internal representation, while still maintaining backwards compatibility.

The implementation is very simple an is essentially a copy from `typing_inspect` with one exception: `ClassVar` was special-cased in `typing_inspect`, but I think this special-casing doesn't really help and only makes things more complicated.
…async (GH-13464)

Automatically replace
tp_print -> tp_vectorcall_offset
tp_compare -> tp_as_async
tp_reserved -> tp_as_async
It seems to be the only widget label not capitalized.
… 2. (GH-11859)

Escape ``\r``, ``\0`` and ``\x1a`` (end-of-file on Windows) in Unicode strings.
…anup. (GH-10320)

TemporaryDirectory.cleanup() failed when non-writeable or non-searchable
files or directories were created inside a temporary directory.
The ssl module now can dump key material to a keylog file and trace TLS
protocol messages with a tracing callback. The default and stdlib
contexts also support SSLKEYLOGFILE env var.

The msg_callback and related enums are private members. The feature
is designed for internal debugging and not for end users.

Signed-off-by: Christian Heimes <[email protected]>
If a type's __ipow__ method was implemented in C, attempting to use
the *modulo* parameter would cause crashes.


https://bugs.python.org/issue36379
@gvanrossum
Copy link
Member

It's not cool to spam the entire team with code review requests.

@jsaporta
Copy link
Author

I synced my fork with master then rebased the issue branch on top of it before updating it. Looks like that may have been a mistake?

@methane
Copy link
Member

methane commented Jun 17, 2019

Please use merge instead of rebase next time.
ref: python/devguide#496

@brettcannon
Copy link
Member

I'm closing this as it has 893 changed files so there's no chance to easily view what the PR is actually targeting.

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.