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

Type inference for max() "tainted" by default #16267

Open
jwodder opened this issue Oct 15, 2023 · 3 comments
Open

Type inference for max() "tainted" by default #16267

jwodder opened this issue Oct 15, 2023 · 3 comments

Comments

@jwodder
Copy link

jwodder commented Oct 15, 2023

Consider the following code:

from __future__ import annotations
from collections.abc import Iterable
from packaging.version import Version

def best_str(strs: Iterable[str]) -> str | None:
    return max(strs, key=keyfunc, default=None)


def keyfunc(s: str) -> str:
    return s[::-1]

I believe this should type-check without issue; in particular, by my reading of max()'s type annotations in typeshed, the key callable should be expected to take only those types contained in the iterable. However, mypy disagrees:

max-arg2.py:6: error: Argument "key" to "max" has incompatible type "Callable[[str], str]"; expected "Callable[[str | None], SupportsDunderLT[Any] | SupportsDunderGT[Any]]"  [arg-type]

Note that mypy expects the key callable to accept str | None even though strs is an Iterable[str]. Moreover, if the default argument is omitted and the return type of best_str() is changed to str, then mypy accepts the code.

Your Environment

  • Mypy version used: 1.6.0
  • Mypy command-line flags: None
  • Mypy configuration options from mypy.ini (and other config files): None
  • Python version used: 3.11.6
@jwodder jwodder added the bug mypy got something wrong label Oct 15, 2023
@AlexWaygood
Copy link
Member

Thanks for the great bug report! It looks like this still repros if you use --new-type-inference, as well.

@ilevkivskyi
Copy link
Member

default is a red herring. This has nothing to do with any fancy inference logic, it is plain old overusing of outer context. I bet if you will rewrite it as temp = max(...); return temp it will work. A while ago I added a special-casing for this for assignments, see #14151, I guess we may want to have matching special-casing for return.

@tamird
Copy link
Contributor

tamird commented Feb 21, 2024

@ilevkivskyi confirmed assigning to a local variable first makes this work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants