-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Add __hash__ to ManimColor #4051
base: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Left one comment, otherwise it should be ready to go.
It would also be nice to have a test for this, maybe something like
def test_color_hash():
assert hash(WHITE) == hash(WHITE.copy())
assert hash(WHITE) != hash(RED)
@@ -1021,6 +1021,9 @@ def __xor__(self, other: Self) -> Self: | |||
self._internal_from_integer(self.to_integer() ^ int(other), 1.0) | |||
) | |||
|
|||
def __hash__(self) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Python docs recommend that __hash__
returns an integer.
Can you just return hash(self.to_hex(with_alpha=True))
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to change the return typehint to int
.
for more information, see https://pre-commit.ci
def test_color_hash():
assert hash(WHITE) == hash(ManimColor([1.0, 1.0, 1.0, 1.0]))
assert hash(WHITE) != hash(RED) Because def test_color_hash():
assert hash(WHITE) == hash("#FFFFFFFF")
assert hash(WHITE) != hash(RED) |
Overview: What does this pull request change?
Adds a hash method to ManimColor
Motivation and Explanation: Why and how do your changes improve the library?
I wanted to use the Color in a dictionary and it didn't work and I thought other people might want the same.
Links to added or changed documentation pages
Further Information and Comments
Reviewer Checklist