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

decouple types.DynamicClassAttribute from property #13276

Merged
merged 6 commits into from
Mar 7, 2025

Conversation

tungol
Copy link
Contributor

@tungol tungol commented Dec 21, 2024

In addition to the cleared allowlist entries, this improves the inheritance of enum.property.

Previous: #12762

This is now enabled by mypy 1.14 and python/mypy#18150.

This comment has been minimized.

stdlib/types.pyi Outdated
def __set__(self, instance: Any, value: Any) -> None: ...
def __delete__(self, instance: Any) -> None: ...
def getter(self, fget: Callable[[Any], Any]) -> DynamicClassAttribute: ...
def setter(self, fset: Callable[[Any, Any], None]) -> DynamicClassAttribute: ...
Copy link
Member

Choose a reason for hiding this comment

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

I think we want the special semantics where you can write @foo.setter later without type checkers complaining about a duplicate definition. That may be hard to achieve without aliasing property.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I'll put together a test to find out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that it works as expected in mypy but not pyright.

This comment has been minimized.

@tungol
Copy link
Contributor Author

tungol commented Dec 22, 2024

I opened a pyright issue for the test failure here.

@tungol
Copy link
Contributor Author

tungol commented Dec 22, 2024

Pyright doesn't plan to support this, so there's nothing else to do here.

@tungol tungol reopened this Dec 23, 2024
@tungol
Copy link
Contributor Author

tungol commented Dec 23, 2024

After looking into it a little more, it seems like we can avoid regressions in pyright by pretending that DynamicClassAttribute inherits from property. That seems less wrong than pretending that it's just an alias for property.

This comment has been minimized.

This comment has been minimized.

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

Thanks, just a nit about the unused return types of callables.

@tungol
Copy link
Contributor Author

tungol commented Mar 7, 2025

As-is, the callable return types match the definition of property in builtins.pyi. object makes sense to me, but should we update property as well if we do that?

@srittau
Copy link
Collaborator

srittau commented Mar 7, 2025

As-is, the callable return types match the definition of property in builtins.pyi. object makes sense to me, but should we update property as well if we do that?

We should probably do that, but considering how fundamental that is, we should do it in a separate PR.

tungol and others added 2 commits March 7, 2025 06:11
Co-authored-by: Sebastian Rittau <[email protected]>
these can go away if we update property itself
Copy link
Contributor

github-actions bot commented Mar 7, 2025

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@srittau srittau merged commit 56fa438 into python:main Mar 7, 2025
55 checks passed
@tungol tungol deleted the DynamicClassAttribute branch March 7, 2025 23:45
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.

3 participants