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

Remove bogus TODO added in #18585 #18603

Merged

Conversation

sterliakov
Copy link
Collaborator

@sterliakov sterliakov commented Feb 5, 2025

Removes todo added in #18585 - such normalization is not technically correct when a tuple is used as a base class.

@sterliakov sterliakov force-pushed the feature/st-normalize-tuple-fallback branch from 558a8f7 to ff7105b Compare February 5, 2025 00:52

This comment has been minimized.

@sterliakov sterliakov force-pushed the feature/st-normalize-tuple-fallback branch from 0a4695d to a3bd40f Compare February 5, 2025 02:14

This comment has been minimized.

@sterliakov sterliakov marked this pull request as ready for review February 5, 2025 03:10
mypy/types.py Outdated Show resolved Hide resolved
@ilevkivskyi
Copy link
Member

Oh, sorry, I completely forgot about one important case where fallbacks can't be ignored: tuple subclasses. For example when one does:

class C(tuple[int, str]):
    ...

we must put tuple[int | str, ...] as an actual base Instance (because tuple is not variadic in typeshed, mostly for historical reasons, and this is unlikely to change).

@ilevkivskyi
Copy link
Member

I think you can repurpose this PR to simply remove the TODO comment.

This comment has been minimized.

@sterliakov
Copy link
Collaborator Author

we must put tuple[int | str, ...] as an actual base Instance

How does/can that manifest and why aren't any testcases failing?

@ilevkivskyi
Copy link
Member

First, all user-defined tuple types get patched in-place after semantic analysis (see calculate_tuple_fallback() and how it is called); second, tuple types are heavily special-cased in many common contexts. So it will probably only manifest in some obscure cases, but even then I don't want to add some code that is misleading.

@sterliakov sterliakov changed the title Always normalize regular tuple fallback to tuple[Any, ...] Remove bogus TODO added in #18585 Feb 10, 2025
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

freqtrade (https://github.com/freqtrade/freqtrade): 1.94x slower (131.3s -> 254.1s in a single noisy sample)

@ilevkivskyi ilevkivskyi merged commit 1edb1d2 into python:master Feb 10, 2025
18 checks passed
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.

2 participants