FIX: Minimize scope for directory changes while loading results file #3061
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oesteban
approved these changes
Oct 2, 2019
Codecov Report
@@ Coverage Diff @@
## master #3061 +/- ##
==========================================
- Coverage 67.72% 67.69% -0.03%
==========================================
Files 344 344
Lines 44087 44091 +4
Branches 5554 5555 +1
==========================================
- Hits 29856 29846 -10
- Misses 13465 13478 +13
- Partials 766 767 +1
Continue to review full report at Codecov.
|
satra
approved these changes
Oct 2, 2019
|
dcm2niix bug cropping up again. Merging master. Please see nipreps/fmriprep#1800 and nipreps/fmriprep#1803 for fixed tests. |
effigies
force-pushed
the
fix/results_file
branch
from
b84e0c6
to
39806c6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR attempts to handle the results file failures by separating concerns.
Load the results file without changing directories. We are pretty consistently seeing "No file named
results_<whatever>.pklz, which at the minimum is uninformative. There's no reason not to know the full path of the file when we try to open it.Perform
pickle.loadson the file contents as a single-linewith indirectorycontext block.All other operations are directory independent and file-lock independent.
Alternative to #3047.
Sorry to preempt that process, but we're seeing this a lot, and I don't really know how to help move that one along. I'm going to use this branch to see if we can resolve these issues that are popping up a lot in the fMRIPrep tests.
Acknowledgment