niworkflows icon indicating copy to clipboard operation
niworkflows copied to clipboard

Consider renaming ``StructuralReference`` interface

Open oesteban opened this issue 3 years ago • 3 comments

Coming out from https://github.com/nipreps/niworkflows/pull/610#discussion_r570260072

Although, I'm still not sure StructuralReference is a bad name. We are indeed calculating "structural" images (as in, with the maximal structural contrast as possible).

If that argument is convincing, we could close this.

oesteban avatar Feb 05 '21 15:02 oesteban

I think we should upstream the fix directly into RobustTemplate, though maybe triggered by a short_circuit boolean input, in case anybody depends on the failure condition. Note that this interface exists because mri_robust_template errors if only passed one image, not that it successfully does something we don't like or wastes time.

effigies avatar Feb 05 '21 15:02 effigies

Sounds good, a just one comment:

  • Add the short_circuit boolean to the outputs too, for the case someone wants to implement different behaviors depending on whether there were one or more images at the input (asking for a friend).

BTW, we should be careful that these tweaks work well with MapNodes. I just discovered that this tweak - https://github.com/nipreps/niworkflows/blob/e2a86a43bd3b4330e9edd0257c422563d2b09f85/niworkflows/interfaces/fixes.py#L87-L133 doesn't work.

oesteban avatar Feb 05 '21 16:02 oesteban

  • Add the short_circuit boolean to the outputs too, for the case someone wants to implement different behaviors depending on whether there were one or more images at the input (asking for a friend).

So short_circuit on input is whether to enable short_circuiting and on output indicates whether the short circuit was used? e.g.,

outputs["short_circuit"] = self.inputs.short_circuit and len(self.inputs.in_files) == 1

I feel like I would rather not use the same name for two subtly different concepts, but as long as I understand what you want, we can quibble about naming when we go to implement.

effigies avatar Feb 05 '21 17:02 effigies