-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Fix/attrs init overload #19104
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
base: master
Are you sure you want to change the base?
Fix/attrs init overload #19104
Conversation
This comment has been minimized.
This comment has been minimized.
I'm afraid this will break horribly down the road. Any custom To see this actually causing trouble (maybe I'm reading the code wrong, after all), please try adding a testcase with such overridden |
Hello! First of all, thank you for your detailed comment. From what I understand, I don’t believe there’s anything wrong with the code I wrote to address the issue. However, based on your feedback, it seems you’re suggesting that instead of returning None, it would be better to explicitly fail when a custom init is detected—something like: (This is my first PR, and English is not my first language, so I really appreciate your understanding in case I misunderstood anything) |
Huh, not exactly, raising a However, on second thought we usually avoid producing error messages when some construct is not supported or cannot be expressed statically. It might be wiser to fall back to Any in that case and accept the call as-is, even if the arguments do not match. I'm not sure which of the options is better. To explain my point about "potential problems" caused by overloaded or even just manually defined from typing import overload
import attrs
@attrs.frozen(init=False)
class C:
x: "int | str"
@overload
def __init__(self, x: int) -> None: ...
@overload
def __init__(self, x: str) -> None: ...
def __init__(self, x: "int | str") -> None:
self.__attrs_init__(x)
obj = C(1)
attrs.evolve(obj, x=2)
attrs.evolve(obj, x="2") # E: ... - False positive import attrs
@attrs.frozen(init=False)
class C:
x: "int | str"
def __init__(self, x: "int | str", y: bool) -> None:
self.__attrs_init__(x)
obj = C(1, False)
attrs.evolve(obj, x=2) # False negative, error at runtime |
Thank you so much for the detailed explanation. It was really helpful and I believe it will greatly assist me in resolving the issue. Among the approaches you suggested, I feel that using I'll revise the code accordingly and push the changes soon. |
for more information, see https://pre-commit.ci
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
|
Fixes #19003
This PR fixes the issue by handling the case where the init method can be an OverloadedFuncDef with CallableType items.