-
Notifications
You must be signed in to change notification settings - Fork 648
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
Explicitly define __hash__ for groups #1398
Conversation
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.
Looks good, except cross Universe checks needed
Also needs CHANGELOG entry
b = getattr(u, level_b)[0:-1] | ||
yield _hash_not_equal, a, b | ||
|
||
|
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.
these tests are good, but you also need to check the cross universe case
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.
Done
|
||
u = make_Universe(size=(3, 3, 3)) | ||
levels = ('atoms', 'residues', 'segments') | ||
for level_a in levels: |
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.
just stylistic, but I'd use itertools here
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.
I spent some time today removing itertools use from cython function. It obviously messed up with my brain.
88ed46b
to
964845c
Compare
I forced pushed an amended commit. I had to fix the reference to the issue in the initial commit message. |
package/CHANGELOG
Outdated
@@ -26,7 +26,7 @@ Changes | |||
|
|||
06/03/17 utkbansal, kain88-de, xiki-tempula, kaplajon, wouterboomsma, | |||
richardjgowers, Shtkddud123, QuantumEntangledAndy, orbeckst, | |||
kaceyreidy | |||
kaceyreidy, jbarnoud |
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.
We're in 16.2 now :p
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.
Done
e0810d9
to
1717886
Compare
Travis full ran into time limit. I restarted it but not sure if it will complete. |
I have a moment... I will rebase against develop and force push. |
Groups (AtomGroup, ResidueGroup, SegmentGroup) cannot be stored in sets or used as dict key if they are not hashable. In python 3, the __hash__ method is not defined implicitly anymore when a class has a __eq__ method. Fixes #1397
1717886
to
e3d5818
Compare
Groups (AtomGroup, ResidueGroup, SegmentGroup) cannot be stored in sets
or used as dict key if they are not hashable. In python 3, the hash
method is not defined implicitly anymore when a class has a eq method.
Fixes #1397
PR Checklist
[ ] Docs?