Skip to content

BUG : add __hash__ to AffineBase #2909

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.

Sign up for GitHub

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

Closed
tacaswell wants to merge 2 commits into from
Closed

BUG : add __hash__ to AffineBase #2909

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c1ab6c6
BUG : add __hash__ to AffineBase
tacaswell Mar 19, 2014
c3bea7a
ENH : tweaked hash method and documentation
tacaswell Mar 19, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
BUG : add __hash__ to AffineBase 
python3 requires that if a class defines __eq__ then in must define
__hash__ (so that if a == b then hash(a) == hash(b)).  Define 64bit
hash by pulling the first 8 characters out of a sha1 hash of the matrix

closes #2828
  • Loading branch information
@tacaswell
tacaswell committed Mar 19, 2014
commit c1ab6c6684e8bddfe0971e6b8d8cd2d66ab5acbe
9 changes: 8 additions & 1 deletion lib/matplotlib/transforms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
unicode_literals)

import six

import hashlib
import numpy as np
from numpy import ma
from matplotlib._path import (affine_transform, count_bboxes_overlapping_bbox,
Expand Down Expand Up @@ -1570,6 +1570,13 @@ def __eq__(self, other):
return np.all(self.get_matrix() == other.get_matrix())
return NotImplemented

# python3 requires that if a class defines __eq__ then in must define

@pelson pelson Mar 19, 2014

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where is that stated? The documentation states that it is perfectly valid for objects to define __eq__ and not __hash__ (http://docs.python.org/3.4/reference/datamodel.html#object.__hash__) as far as I can see.

@tacaswell tacaswell Mar 19, 2014

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

They must if they want to be hashable, clarified the comment.

@pelson pelson Mar 19, 2014

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do transforms want to be hashable? They aren't currently AFAICT?

# __hash__ (so that if a == b then hash(a) == hash(b)). Define 64bit
# hash by pulling the first 8 characters out of a sha1 hash of the matrix
# see issue #2828
def __hash__(self):
return int(hashlib.sha1(self.get_matrix()).hexdigest()[:8], 16)

@mdboom mdboom Mar 19, 2014

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems like a correct approach. But I wonder if it's faster to do:

hash(self.get_matrix().tostring())

(Don't know if it is, just though the code would spend more time in C that way...)

@tacaswell tacaswell Mar 19, 2014

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

indeed

In [51]: tt = np.random.rand(3, 3)

In [52]: %timeit int(hashlib.sha1(tt).hexdigest()[:8], 16)
1000000 loops, best of 3: 1.58 µs per loop

In [53]: %timeit hash(tt.tostring())
1000000 loops, best of 3: 270 ns per loop


def transform(self, values):
return self.transform_affine(values)
transform.__doc__ = Transform.transform.__doc__
Expand Down