Skip to content

Conversation

@sobolevn
Copy link
Member

@sobolevnsobolevn commented Aug 2, 2022

While working on stubs for django I've noticed that sometimes people redefine __hash__.

But, this is not what we do for other methods. For example, if base class A defines foo, its child class B does not need to also define foo. Parent's definition will be used by default. We only annotate redefinitions.

So, __hash__is defined in object. Child classes may not redefine it, unless:

  • __hash__ returns something different, not int (like enum.Enum does right now)
  • __hash__ is set to be ClassVar[None]
  • __hash__ is reset after parent's ClassVar[None] to indicate that subclass is immutable again
  • __hash__ is required to be set explicitly as a part of @abstractmethod parent's def, like numbers.Number does

Mypy sample: https://mypy-play.net/?mypy=latest&python=3.10&gist=cef2a15be8adfa8d646d260232e6b38a

fromtypingimportHashableclassMyHash: def__hash__(self) ->int: ... classMyClass: ... defrequires_hash(o: Hashable): ... requires_hash(MyHash()) # okrequires_hash(MyClass()) # ok

What do others think?

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Copy link
Collaborator

@srittausrittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, although I'd like another maintainer to confirm.

Copy link
Member

@AlexWaygoodAlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM too, but I'd like to get @JelleZijlstra's feedback. I think he had some Thoughts when I tried to do something along these lines a while back with some other methods.

@sobolevn
Copy link
MemberAuthor

@JelleZijlstra frienly ping 🙂

@AlexWaygoodAlexWaygood merged commit 64bc059 into python:masterAug 6, 2022
@AlexWaygoodAlexWaygood mentioned this pull request Jan 6, 2023
AlexWaygood added a commit to AlexWaygood/typeshed that referenced this pull request Jul 9, 2023
JelleZijlstra pushed a commit that referenced this pull request Jul 9, 2023
…10426) Reverts #8465Fixes#10424Closes#10425#8465 caused regressions: see #10424 and python/mypy#13800. Since it didn't fix any known problems (just some stylistic nits that we had), let's just revert the PR.
Sign up for freeto 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.

4 participants

@sobolevn@srittau@JelleZijlstra@AlexWaygood