Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork3.1k
Add redundant-annotation warning#20238
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 ourterms of service andprivacy 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?
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
A5rocks left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm not completely sure this feature is fine. I thought it was, but when bringing it up in conversation, others pointed out that redefinition might depend on whether there's an annotation. Obviously mypy doesn't implement that algorithmnow, but it might be nice to eventually have it? Or at least have the possibility of having it? And this feature would mean we can't.
To be concrete, I'm referring to how pyright/ty assume a declaration with a type (a: int = 5) can't be redefined but a declaration without (a = 5) can.
| class f: | ||
| g: int = 1 | ||
| [builtins fixtures/tuple.pyi] | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Could you also test:
x: Literal[1] = 1y: list[str] = []- this:
fromtypingimportTypeVardeff(x:T)->T:returnxx:Literal[1]=f(1)y:list[str]=f([])
And uh, I guess it would be nice to test a case where running type checking without type context would error. (you may need to silence errors for a specific run?)... unfortunately I cannot think of any examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
And this feature would mean we can't.
If that redefinition algorithm is implemented, couldn't this flag be mutually exclusive with the redefinition flag?
Could you also test:
Tests added (commitebf40a5). In the TypeVar test, it looks like is_same_type does not handle this. May be related to#19761
test a case where running type checking without type context would error
Does that mean a test like:
# mypy: check-untyped-defsdeff():return4defg():j:int=f()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If that redefinition algorithm is implemented, couldn't this flag be mutually exclusive with the redefinition flag?
Yeah, potentially. I don't think that would be good UX...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Does that mean a test like:
No, there's been issues like#20013 where part of the issue is that we're running type inference twice (once without type context for heuristics reasons, and once with) and the one without type context is erroring.
I can't think of any small reproducer because it would be a bug in mypy I think...
This comment has been minimized.
This comment has been minimized.
grayjk commentedNov 17, 2025
When a= (None, []) that conflicts with the warning from |
A5rocks commentedNov 18, 2025
@JukkaL do you have opinions on how this interacts with (maybe) redefinition? I forgot what heuristics you've mentioned using for it before. |
grayjk commentedNov 21, 2025
Testing this on an internal project that usessphinx-autodoc-typehints, this branch warns on a redundant type hint that is meant for the sphinx generated documentation. An unfortunate downside |
sterliakov commentedNov 24, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I'd say this check shouldonly fire for function locals. Redundant annotations at top level (globals) and class members should be fine - they declare the public interface explicitly. If I say STEPS_LIMIT:int=3 at top level, I want it to mean "this is a max count of steps to perform, some integer, but make no assumptions about its value". It should not be reinterpreted as final or literal, for example, and the explicit type hint means "yes, really an int, it is not going to become a float later without semver-major update, but its value could change to 20 in a patch release". This would also resolve your sphinx-autodoc-typehints issue, I believe? |
grayjk commentedNov 24, 2025
Updated to this |
According tomypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Uh oh!
There was an error while loading.Please reload this page.
Relates to#18540
Adds a warning for redundant-annotation where a function local variable annotation type is the same as the inferred type
Based heavily on the code from@asottile in the issue