ENH: Add "ExportFile" interface as simple alternative to "DataSink"

[stilley2]

Acknowledgment

• (Mandatory) I acknowledge that this contribution will be available under the Apache 2 license.

[effigies]

Hi @stilley2, could you clarify the reasoning here? Is the idea to use this as a kind of cheap DataSink? In general, symlinking should be a safe operation within the workflow.

[stilley2]

Hi @effigies , yeah that's exactly what I was thinking. Specifically it could be used when output file name generation is handled by an external program (e.g. pybids). Perhaps a new interface would be clearer than hijacking Rename for this purpose?

[effigies]

Perhaps a new interface would be clearer than hijacking Rename for this purpose?

I think so. It may be that DataSink will actually be enough if it's a single file, but you could easily write a simple ExportFile (or whatever name seems appropriate) interface that really does just copy one file with a given name.

Given that you're looking at using pybids, you could also check out niworkflows.interfaces.bids.DerivativesDataSink or fitlins.interfaces.bids.BIDSDataSink, to see if those meet your needs. It might be time to consider upstreaming these to nipype, if they're usable outside our specific contexts.

[effigies]

Thanks for this. I've made some suggestions. Another thing you can look into to avoid using _list_outputs in addition to _run_interface is SimpleInterface.

[codecov]

Codecov Report

Merging #3054 into master will decrease coverage by 3.61%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##           master    #3054      +/-   ##
==========================================
- Coverage   68.19%   64.58%   -3.62%     
==========================================
  Files         297      295       -2     
  Lines       39771    39712      -59     
  Branches     5210     5202       -8     
==========================================
- Hits        27123    25648    -1475     
- Misses      11939    13025    +1086     
- Partials      709     1039     +330

Flag	Coverage Δ
#smoketests	?
#unittests	64.58% <91.66%> (-0.86%)	⬇️

Impacted Files	Coverage Δ
nipype/utils/filemanip.py	75.38% <100%> (-3.56%)	⬇️
nipype/interfaces/io.py	50.15% <90%> (-9.11%)	⬇️
nipype/interfaces/nilearn.py	40% <0%> (-56.67%)	⬇️
nipype/workflows/__init__.py	47.05% <0%> (-52.95%)	⬇️
nipype/utils/spm_docs.py	25.92% <0%> (-44.45%)	⬇️
nipype/interfaces/freesurfer/base.py	46.61% <0%> (-32.21%)	⬇️
nipype/utils/logger.py	59.7% <0%> (-29.86%)	⬇️
nipype/algorithms/rapidart.py	35% <0%> (-29.42%)	⬇️
nipype/interfaces/spm/base.py	57.94% <0%> (-29.14%)	⬇️
nipype/utils/provenance.py	55.73% <0%> (-28.35%)	⬇️
... and 38 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1725f51...3695d36. Read the comment docs.

[effigies]

Ah, you have a FileExistsError, which didn't exist prior to Python 3.5. If you want to use it, could you add it to utils.filemanip, along with FileNotFoundError?

nipype/nipype/utils/filemanip.py

Lines 43 to 52 in 1ccd70b

	try:
	from builtins import FileNotFoundError
	except ImportError: # PY27
	class FileNotFoundError(OSError): # noqa
	"""Defines the exception for Python 2."""
	def __init__(self, path):
	"""Initialize the exception."""
	super(FileNotFoundError, self).__init__(
	2, 'No such file or directory', '%s' % path)

There's also a Python 3.5 issue, which I suspect will be fixed if you merge/rebase master. I would certainly try that before we start digging deeper.

[effigies]

I think this will resolve the Python 3.5 issue. I don't think we need to obsess over what happens if people pass pathlib2.Paths as File traits on Python 3.5.

[effigies]

Thanks for this. Some suggestions. I think the test failure is related to output_folder...

[effigies]

This file needs to exist. By default, all doctests are run in nipype/testing/data, so there are a lot of options to pick from.

[effigies]

This feels like an unnecessary qualifier. I think you could probably just skip this text, unless you want to have multiple tests showing off check_extension and clobber. If so, you could say "By default, file extensions need to match, and the output file should not exist:" and something similar above the other examples.

[stilley2]

Thanks @effigies . Sorry for the delay. Busy week.

[effigies]

No worries. This looks good. Two minor issues should fix up the tests, I think.

[effigies]

Can you run make specs and commit nipype/interfaces/tests/test_auto_ExportFile.py?