DerivativesDataSink datatype coercion does not skip non-dtseries CIFTIs

[tsalo]

I have been trying to write out a dseg-suffixed CIFTI file with the .dlabel.nii extension with DerivativesDataSink, but it raises the following exception:

Traceback:
	Traceback (most recent call last):
	  File "/usr/local/miniconda/lib/python3.8/site-packages/nipype/interfaces/base/core.py", line 398, in run
	    runtime = self._run_interface(runtime)
	  File "/usr/local/miniconda/lib/python3.8/site-packages/niworkflows/interfaces/bids.py", line 712, in _run_interface
	    new_header.set_data_dtype(data_dtype)
	AttributeError: 'Cifti2Header' object has no attribute 'set_data_dtype'

In the following section, DerivativesDataSink checks to see if an output file is a NIFTI by looking at the extension. However, it only counts .dtseries.nii[.gz] files as CIFTIs, and assumes any other files ending in .nii[.gz] are NIFTIs.

niworkflows/niworkflows/interfaces/bids.py

Lines 651 to 653 in 1555d2b

	is_nifti = out_file.name.endswith(
	(".nii", ".nii.gz")
	) and not out_file.name.endswith((".dtseries.nii", ".dtseries.nii.gz"))

After that, DerivativesDataSink attempts to coerce the in_file's datatype to a target type for that suffix. However, this step only works for NIFTIs, so any CIFTIs mislabeled as NIFTIs above will cause an error.

I can't think of a great general-purpose solution, but two ideas I had were:

1. Hardcode the full list of CIFTI extensions in the check. The extensions might need to be updated from time to time though.

2. In the following section, check for a nii.nifti_header attribute, and set the datatype for that attribute instead of nii.header when it is present.

   niworkflows/niworkflows/interfaces/bids.py

   Lines 697 to 712 in 1555d2b

   	if data_dtype:
   	data_dtype = np.dtype(data_dtype)
   	orig_dtype = nii.get_data_dtype()
   	if orig_dtype != data_dtype:
   	LOGGER.warning(
   	f"Changing {out_file} dtype from {orig_dtype} to {data_dtype}"
   	)
   	# coerce dataobj to new data dtype
   	if np.issubdtype(data_dtype, np.integer):
   	new_data = np.rint(nii.dataobj).astype(data_dtype)
   	else:
   	new_data = np.asanyarray(nii.dataobj, dtype=data_dtype)
   	# and set header to match
   	if new_header is None:
   	new_header = nii.header.copy()
   	new_header.set_data_dtype(data_dtype)

[mgxd]

Hardcode the full list of CIFTI extensions in the check. The extensions might need to be updated from time to time though.

This is implemented in nipy/nibabel#932, might be time to push that over the hill.

Alternatively, we could add a new input field that allows developers to set the output image class, with it defaulting to "infer" or something.

[effigies]

This is one reason I don't love having a massive one-size-fits-all DerivativesDataSink in niworkflows, because it's really hard not to tailor it to the use case of fMRIPrep.

Possibly the most foolproof check is:

try:
    img = nb.load(in_file)
except:
    # Not an image
else:
    is_nifti = isinstance(img, Nifti1Image)
    is_cifti = isinstance(img, Cifti2Image)
    ...

It's not expensive to load a header. We shouldn't be mucking about inferring types from extensions.