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

Should dict() constructor overloads be more permissive? #11532

Open
dimaqq opened this issue Mar 5, 2024 · 5 comments
Open

Should dict() constructor overloads be more permissive? #11532

dimaqq opened this issue Mar 5, 2024 · 5 comments

Comments

@dimaqq
Copy link

dimaqq commented Mar 5, 2024

Current constructor overloads are here:

typeshed/stdlib/builtins.pyi

Lines 1041 to 1046 in e80ad6b

# Next two overloads are for dict(string.split(sep) for string in iterable)
# Cannot be Iterable[Sequence[_T]] or otherwise dict(["foo", "bar", "baz"]) is not an error
@overload
def __init__(self: dict[str, str], __iterable: Iterable[list[str]]) -> None: ...
@overload
def __init__(self: dict[bytes, bytes], __iterable: Iterable[list[bytes]]) -> None: ...

Someone took care to allow specific positive case, and to block specific negative case.
Many potential valid uses have fallen through the cracks:

dict([[1, 2]])
dict("AT TA GC CG".split())
dict([["str", b"bytes"]])

My understanding of the issue is that there's no way to specify sequence length (other than a tuple) in a type hint, that is there's no syntax to hint ["a", "b"] vs ["a", "b", "c"].

This brings a philosophical question: what side should typeshed err on when type hint syntax is not precise enough?

  • type whatever Python may accept run time, or
  • restrict users to what Python is guaranteed to accept at run time?

It was mentioned at microsoft/pyright#7382 that type checkers trust typeshed, and thus the question belongs here.

My personal preference would be for permissive type hints in these cases.

@srittau
Copy link
Collaborator

srittau commented Mar 5, 2024

Previously discussed in #2287. This is due to the lack of fixed-length sequences, see python/typing#592.

@dimaqq
Copy link
Author

dimaqq commented Mar 6, 2024

Indeed, than you for the reference to the canonical upstream ticket.

Come to think of it, it seems there's already a precedent for looser overload spec:

typeshed/stdlib/builtins.pyi

Lines 1043 to 1044 in e80ad6b

@overload
def __init__(self: dict[str, str], __iterable: Iterable[list[str]]) -> None: ...

Typing machinery allows both of the below, when only one of the two is allowed at runtime:

dict([["a", "b"]])
dict([["a", "b", "c"]])

I suppose the wider discussion should involve type checker folks and maybe some proxy for user code, but I'm not sure where this discussion should take place, in this repo issue, or someplace else?

@JelleZijlstra
Copy link
Member

This repo is the right place to make that decision. If you make PR with a change, the mypy-primer tool will test its effect on a sample of open-source code.

@Akuli
Copy link
Collaborator

Akuli commented Mar 6, 2024

There is more discussion about this in #10013.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Mar 6, 2024

Note that mypy_primer is much better at measuring effect of changes that produce false positives than changes that produce false negatives. You're subject to survivorship bias [insert aeroplane meme here] and reliant on unused ignores.

To your philosophical question, typeshed will usually prefer false negatives over false positives. That said, as the existence of the str splitting overloads indicate, we often try to be pragmatic.

With that in mind, and that three issues about this in six years isn't the worst thing, I'd like to not have the fully general Iterable[Iterable[T]] overload. In particular, I'd like to keep the true positives on things that manifest in cases where str being a Sequence[str] comes up. Maybe something like def __init__(self: dict[T, T], __iterable: Iterable[list[T]]) -> None: ... would work? What is the specific real world false positive you're running into?

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

No branches or pull requests

5 participants