-
Notifications
You must be signed in to change notification settings - Fork 532
ENH: Stop printing false positive differences when logging cached nodes #3376
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
I don't think the failing test is related, right? |
This comment has been minimized.
This comment has been minimized.
238d1c2
to
a30018e
Compare
5cd45ad
to
90e2130
Compare
@oesteban Heads up that I rebased. |
Codecov Report
@@ Coverage Diff @@
## master #3376 +/- ##
==========================================
+ Coverage 64.88% 64.95% +0.07%
==========================================
Files 307 307
Lines 40365 40365
Branches 5337 5339 +2
==========================================
+ Hits 26189 26220 +31
+ Misses 13081 13049 -32
- Partials 1095 1096 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
90e2130
to
6bd611f
Compare
6bd611f
to
1beec59
Compare
The function calculating changed input fields did not preempt the problem of comparing tuples vs. lists in most of the cases. This PR simplifies the logic of the
dict_diff
accessory function, and fixes the issue - resulting in way shorter indices of changed inputs (esp. when an interface has several inputs and some of them contain long lists).This issue translated in practice into showing long lists of unchanged values as if they had changed, when often, the changed input invalidating the cache would be some other field.
EDIT: Beyond the more robust implementation (former was kind of brittle), this PR is useful, e.g., when re-running fMRIPrep and large lists of files have changed (e.g., the inputs to an fslmerge node with a list of 800 input files).