Add bound Hashable for types for keys of type Mapping and values of type Set

kruvasyan commented 1 year ago

Problem

Types used for keys in mappings and values in sets are too broad.

How to reproduce

d: [list, int] = {}  # Tools that use typeshed see this code as valid.
d[[1]] = 2 # TypeError: unhashable type: 'list'

Solution

Change type of keys in mappings: _KT = TypeVar("_KT") to _KT = TypeVar("_KT", bound=Hashable)

[x] Added PR with fixes to stdlib

But I still have questions about how to fix the types in set and frozenset.

For set, we using _T, which is also used for values in the list. Can I change it to _KT? The requirements for the type of values in the set are the same as for the keys in the dictionary. I understand that it is a value, not a key, and the name can be confusing. Do you think of a better option?
For frozenset, we using covariant _T_co, which is also used for values in the tuple. It's the same problem here. We need a new type that is covariant and bound to Hashable.
[ ] set
[ ] frozenset

And once we figure out what to do with types in stdlib, I will be happy to add the same changes to third-party libraries.

AlexWaygood commented 1 year ago

This has been tried before, and it was too disruptive:

6244

But it's always good to try these things again every so often, to see if type-checker support has improved in the meantime. So, thanks for opening the issue!

kruvasyan commented 1 year ago

Oh, I saw. Can I then convert this issue to a tracking issue and list the packages having problems? Then step by step, we can fix them all.

hauntsaninja commented 1 year ago

We care about mypy_primer output not because we care about those projects specifically, but because we assume it will be representative of all the code out there that is being type checked.

Our goal is to ensure a high signal to noise ratio on type checker errors. If a change creates a bunch of new errors that do not add much value, we don't accept that change, because it will cause a bunch of noisy new errors on all typed code out there.

kruvasyan commented 1 year ago

Yes, I realize that's just a few errors, and the output contains only popular projects.

If we look at the errors, they are primarily due to the use of Any. And it looks like we will always be penalized in the future for using Any. Because we use Any as a mute for the needed type until it exists. But when the required type appears or we have worked out how to express it, adding it adds errors. This sounds a bit hopeless because, over time, it's harder and harder to add new types :)

Thank you for your comprehensive answers!

JukkaL commented 1 year ago

This sounds a bit hopeless because, over time, it's harder and harder to add new types :)

Yes, it will harder to improve types when there is lots of existing code. On the other hand, any improvements will have a bigger impact, since there are many users who can benefit.

JelleZijlstra commented 1 year ago

Note that the vast majority of the mypy-primer errors are because mypy apparently doesn't accept type as Hashable. Fixing that one bug should make the situation much better, though I suspect there's still going to be more issues.

AlexWaygood commented 1 year ago

I've removed the checklist of third-party projects from the first comment in this issue, as I don't want typeshed contributors filing PRs at all of those projects attempting to "fix" this issue. As @JelleZijlstra, the necessary precondition for changing typeshed's stubs here isn't to fix various third-party projects; it's to improve mypy's understanding of hashability. For now, there are too many false-positives emitted on third-party code to consider making this change.

python / typeshed