-
Notifications
You must be signed in to change notification settings - Fork 532
[FIX] Bugs found after #1572 and #1591 #1623
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
Conversation
Improved the way interfaces were initialized to prevent that lists of lists of int are not set. Fixes nipy#1625
|
||
""" | ||
|
||
reg = Registration(from_file='./smri_ants_registration_settings.json') | ||
reg = Registration(from_file=example_data('smri_ants_registration_settings.json')) |
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.
moved this settings file from examples to the example data, since now I use it in tests
Current coverage is 70.88% (diff: 94.66%)@@ master #1623 diff @@
==========================================
Files 1020 1020
Lines 51240 51277 +37
Methods 0 0
Messages 0 0
Branches 7259 7265 +6
==========================================
+ Hits 36314 36347 +33
- Misses 13840 13849 +9
+ Partials 1086 1081 -5
|
@satra please have a look, this is ready to merge |
dict_withhash.append((name, | ||
self._get_sorteddict(val, True, hash_method=hash_method, | ||
hash_files=hash_files))) | ||
for name, val in sorted(self.get_traitsfree().items()): |
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 reason i used get
in the previous version was to speed up the hashval computation. since get_traitsfree
requires a dictionary cleanup. if using get doesn't change assumptions here, it may be cleaner.
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.
Ok, back to the original 👍
@satra, @chrisfilo this is ready, and fixes a couple of well-identified problems with the python 2/3 compatibility. I have added 3 tests based on ants.Registration to make sure we cover the (possibly) most complex case of nipype interfaces. |
Found corner cases that failed when calculating the hash of dicts - fixes #1620.
Also found that list of lists of ints do not work in #1591.