8000 Fix/attrs init overload by uko3211 · Pull Request #19104 · python/mypy · GitHub
[go: up one dir, main page]

Skip to content

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

uko3211
Copy link
@uko3211 uko3211 commented May 18, 2025

Fixes #19003

This PR fixes the issue by handling the case where the init method can be an OverloadedFuncDef with CallableType items.

This comment has been minimized.

@sterliakov
Copy link
Collaborator

I'm afraid this will break horribly down the road. Any custom __init__ method is a disaster for this plugin, note how we use its arg_names and arg_types to get the actual field types. If __init__ method is customized, this is no longer true, and _get_expanded_attr_types will no longer be correct. I'd suggest just failing with a different error ("overridden init not supported" instead of "not an attrs class"), but also feel free to explore alternative ways to gather fields!

To see this actually causing trouble (maybe I'm reading the code wrong, after all), please try adding a testcase with such overridden __init__ where arg names/types do not correspond to the generated signature and using attrs.evolve on that class.

@uko3211
Copy link
Author
uko3211 commented May 18, 2025

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:
raise TypeError("Custom __init__ methods are not supported with this plugin")
Is that correct?

(This is my first PR, and English is not my first language, so I really appreciate your understanding in case I misunderstood anything)

@sterliakov
Copy link
Collaborator

Huh, not exactly, raising a TypeError is a bit too harsh: mypy reports errors differently, we do not make it crash when a problem is detected in user's code. Look at how _fail_not_attrs_class function reports the problem - I suspect you need to do something similar based on ctx.api.fail.

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 __init__, here are two snippets:

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

@uko3211
Copy link
Author
uko3211 commented May 19, 2025

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 ctx.api.fail might actually introduce unnecessary limitations for mypy users. So I think falling back to Any, as you mentioned, would be a better solution in this case.

I'll revise the code accordingly and push the changes soon.

Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@uko3211
Copy link
Author
uko3211 commented May 19, 2025
  1. The overridden custom init no longer returns an error.
  2. The ambiguous type in the overridden custom init is handled by returning any, allowing users to proceed with awareness of this behavior.

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.

Overloaded __init__ prevents type from being recognized as an attrs class
2 participants
0