[FIX] Compression in DataSinks #1095
Conversation
This PR tries to fix nipreps#1075 for good.
| src_open = gzip.open if src_isgz else open | ||
| dst_open = gzip.open if dst_isgz else open | ||
| with src_open(src, 'rb') as f_in: | ||
| with dst_open(dst, 'wb') as f_out: | ||
| copyfileobj(f_in, f_out) | ||
| return True |
There was a problem hiding this comment.
Unless there's a use for True (and some case in which we might return False), I would just use return with no argument.
There was a problem hiding this comment.
We could add one output for debugging: return False if no compress/uncompress was performed, True otherwise. WDYT?
There was a problem hiding this comment.
Sure. Maybe we should go ahead and add a unit test for this.
There was a problem hiding this comment.
(I recognize that I'm using "we" somewhat disingenuously. I can't stop.)
|
I think |
fmriprep/interfaces/bids.py
Outdated
| @@ -170,6 +170,9 @@ class DerivativesDataSinkInputSpec(BaseInterfaceInputSpec): | |||
|
|
|||
| class DerivativesDataSinkOutputSpec(TraitedSpec): | |||
| out_file = OutputMultiPath(File(exists=True, desc='written file path')) | |||
| compression = traits.List( | |||
| traits.Bool, desc='whether ``in_file`` was compressed/uncompressed ' | |||
| 'or `it was copied directly.') | |||
There was a problem hiding this comment.
I would just use OutputMultiPath, here. There's no good reason to make it a list when out_file isn't.
oesteban
changed the title
fmriprep/interfaces/bids.py
Outdated
| @@ -256,6 +256,9 @@ def _run_interface(self, runtime): | |||
| self._results['out_file'].append(out_file) | |||
| self._results['compression'].append(_copy_any(fname, out_file)) | |||
|
|
|||
| if len(self._results['compression']) == 1: | |||
| self._results['compression'] = self._results['compression'][0] | |||
There was a problem hiding this comment.
OutputMultiPath automatically does this.
This PR tries to fix #1075 for good.