Add missing type hint for AbstractSet._hash() #7153
Conversation
|
Diff from mypy_primer, showing the effect of this PR on open source code: bidict (https://github.com/jab/bidict)
+ bidict/_frozenbidict.py:36: error: Unused "type: ignore" comment
edgedb (https://github.com/edgedb/edgedb)
+ edb/common/checked.py:335:9: error: Cannot assign to a method
+ edb/common/checked.py:335:22: error: Incompatible types in assignment (expression has type "int", variable has type "Callable[[], int]")
+ edb/common/checked.py:338:12: error: Non-overlapping equality check (left operand type: "Callable[[], int]", right operand type: "Literal[-1]")
+ edb/common/checked.py:339:13: error: Cannot assign to a method
+ edb/common/checked.py:339:26: error: Incompatible types in assignment (expression has type "int", variable has type "Callable[[], int]")
+ edb/common/checked.py:340:16: error: Incompatible return value type (got "Callable[[], int]", expected "int")
|
|
@elprans @1st1 in case of interest – looks like (All the best with the upcoming 1.0 launch btw, can't wait to try out EdgeDB sometime!) |
There was a problem hiding this comment.
This isn't ideal, because type checkers think that the built-in set inherits from AbstractSet and also has a _hash() method. At runtime, set doesn't actually inherit from AbstractSet though. So code like set()._hash() will type check but fail at runtime.
According to our CONTRIBUTING.md, we usually prefer false negatives (no error for incorrect code, this PR) over false positives (errors for correct code, current situation). But before merging this, I'll leave this open for a while, so other maintainers can decide what to do with this.
Thanks for the heads up! Yes, the shadowing isn't intentional, will fix.
Thank you! |
collections.abc.Setprovides a_hash()method (see https://github.com/python/cpython/blob/f20ca766/Lib/_collections_abc.py#L677) that includes the following in its docstring:Following this instruction currently causes mypy to give an
attr-definederror becauseAbstractSetis missing a type hint for this_hash()method.I've been hitting this for a while in my bidict library, whose
frozenbidict.__hash__()returnsItemsView(self)._hash(), and working around it with atype: ignorecomment, but I believe the fix is as simple as this PR.Thanks for your consideration.