Skip to content

Conversation

@sobolevn
Copy link
Member

@sobolevn sobolevn commented Jan 24, 2022

@serhiy-storchaka proposed removing __class_getitem__ from PurePath.
CC @asvetlov and @isidentical as original PR authors: #17498

Why?

  1. It is not used anywhere in our code (obviously)
  2. It is not really generic, because PurePath always works with str (unlike os.PathLike[bytes | str]
  3. In typeshed PurePath is not generic: https://github.com/python/typeshed/blob/35064a7f759facd7c3787ab6095964008f97d873/stdlib/pathlib.pyi#L20

Related #30822

CC @corona10 as my mentor

https://bugs.python.org/issue46483

@@ -0,0 +1 @@
Remove ``__class_getitem__`` from :class:`pathlib.PurePath`
Copy link
Member

Choose a reason for hiding this comment

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

It might be good to:

  1. Give a one-sentence description of why it's being removed.
  2. Delete the news entry that was added in bpo-46483: change PurePath.__class_getitem__ to return GenericAlias #30822 (it'll make for a pretty confusing changelog otherwise).

@@ -0,0 +1 @@
Remove ``__class_getitem__`` from :class:`pathlib.PurePath`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Remove ``__class_getitem__`` from :class:`pathlib.PurePath`
Remove ``__class_getitem__`` from :class:`pathlib.PurePath`.

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
@serhiy-storchaka
Copy link
Member

Please add also a What's New entry (in the Removed section).

I think in this case we can cut the deprecation period, as this feature was added by mistake and was unlikely used in user code. But it is still a breaking change which should be documented.

@sobolevn
Copy link
Member Author

sobolevn commented Feb 3, 2022

Thank you! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants