MRtrix3 / mrtrix3

MRtrix3 provides a set of tools to perform various advanced diffusion MRI analyses, including constrained spherical deconvolution (CSD), probabilistic tractography, track-density imaging, and apparent fibre density
http://www.mrtrix.org
Mozilla Public License 2.0
294 stars 181 forks source link

Advanced integer sequence capability #2952

Open Lestropie opened 3 months ago

Lestropie commented 3 months ago

This has been discussed, but I couldn't find it listed anywhere.

Some more advanced capabilities of C++ ".type_sequence_int()" argument types, such as non-unity strides and the ":end" indicator, became broken on dev at some point.

Some of this may be due to #2678, where integer sequences need to be resolvable at command-line parsing time. So where previously such designators would have just been parsed by argparse as strings and fed as-is to any executed binaries, now they'll fail as they can't be directly converted from a string to an integer sequence in the absence of knowledge of the underlying image being operated on. However I'm not 100% sure if this is the only problem in this context.

Contemplating it as I document it, I think that type_sequence_int() is currently not fit for purpose. There needs to be a second CLI type. The C++ documentation currently states:

//! specifies that the argument should be a sequence of comma-separated integer values
Argument &type_sequence_int() {

, yet uses like "mrconvert -coord 0:2:end" clearly don't conform to such. Conversely, there are other command-line arguments for which the "end" designator makes no sense; say for instance a list of lmaxes.

So my current instinct is that we need to implement, for both C++ and Python CLIs, something like "type_sequence_index", which supports non-unity strides and the "end" specifier, whereas "type_sequence_int" must be "a sequence of comma-separated integer values". For Python, this could be resolved at command-line parsing time in the absence of "end", or retained as a string in the presence of "end".