Skip to content

bpo-46162: make property generic #30238

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

bpo-46162: make property generic #30238

wants to merge 6 commits into from

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Dec 23, 2021 •
edited
Loading

@sobolevn sobolevn mentioned this pull request Dec 23, 2021
Closed
4 tasks
kumaraditya303
kumaraditya303 approved these changes Dec 23, 2021
View reviewed changes
Copy link
Contributor

@kumaraditya303 kumaraditya303 left a comment

Choose a reason for hiding this comment

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

LGTM but the PEP should be changed first to account for this change.

@sobolevn
Copy link
Member Author

sobolevn commented Dec 23, 2021 •
edited
Loading

@kumaraditya303 this was my exact question. Because, current version of PEP states:

This is why starting with Python 3.9, the following collections become generic using class_getitem() to parameterize contained types:

I have several issues with that:

  • Python3.9 won't support (without __future__ import) generic property[] as far as I understand (I might be wrong here)
  • property is not a "collection" (as many other types there)

CC @ambv as PEP's author 🙂

@isidentical
Copy link
Member

LGTM but the PEP should be changed first to account for this change.

@kumaraditya303 I am not sure how did you end up with this idea. PEP's are historical documents, and they don't get updated as the language grows. There were multiple occurrences of generic support for classes that the PEP initially did not target (the most recent one I recall is graphlib.TopologicalSorter).

@sobolevn You can simply wait for a few core developers to respond, or just post this on the python-dev (since this is more of a change in the builtins).

isidentical
isidentical reviewed Dec 23, 2021
View reviewed changes
Lib/test/test_property.py Outdated
Comment on lines 217 to 230
def test_property___class_getitem__(self):
from types import GenericAlias
p = property[int, str]
self.assertIsInstance(p, GenericAlias)
self.assertIs(p.__origin__, property)
self.assertEqual(p.__args__, (int, str))
self.assertEqual(p.__parameters__, ())

from typing import TypeVar
G = TypeVar('G')
S = TypeVar('S')
p1 = property[G, S]
self.assertEqual(p1.__args__, (G, S))
self.assertEqual(p1.__parameters__, (G, S))
Copy link
Member

Choose a reason for hiding this comment

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

Please see Lib/test_genericalias.py as well for the tests.

@kumaraditya303
Copy link
Contributor

I am not sure how did you end up with this idea. PEP's are historical documents, and they don't get updated as the language grows. There were multiple occurrences of generic support for classes that the PEP initially did not target (the most recent one I recall is graphlib.TopologicalSorter).

This changes property which is in the builtins which is very widely used and it not in other module so it should be mentioned in the PEP under 3.11 changes for maximum visibility about this change.

@isidentical
Copy link
Member

This changes property which is in the builtins which is very widely used and it not in other module so it should be mentioned in the PEP under 3.11 changes for maximum visibility about this change.

We generally do not change old PEPs, especially when they are in the Final state. A new discussion can (or should [depending on the complexity of this feature]) be brought up on python-dev, though for a case like this I'd personally suggest just waiting the final resolution in python/typing#985 and also the responses of other developers interested in typing before making an action towards python-dev since it might evolve a bit (which I personally do not expect).

merwok
merwok reviewed Dec 31, 2021
View reviewed changes
Misc/NEWS.d/next/Library/2021-12-23-12-53-34.bpo-46162.bE3xrz.rst
@@ -0,0 +1 @@
Make :class:`property` generic.
Copy link
Member

Choose a reason for hiding this comment

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

generic can be a bit vague (or generic 😉), could you rephrase this news snippet and the PR title to clarify this is about typing generic alias?

@sobolevn sobolevn closed this Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@merwok merwok merwok left review comments

@isidentical isidentical isidentical left review comments

@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes

Assignees
No one assigned
Labels
awaiting core review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sobolevn @isidentical @kumaraditya303 @merwok @the-knights-who-say-ni @bedevere-bot