Mouse-Imaging-Centre / pydpiper

Python code for flexible pipeline control
Other
25 stars 10 forks source link

Inconsistent naming of MAGeT protocol options breaks config file usage #347

Open bcdarwin opened 6 years ago

bcdarwin commented 6 years ago

(was https://github.com/Mouse-Imaging-Centre/RMINC/issues/191)

gdevenyi:

I was setting protcol settings in my config file ahead of time. This was fine for MBM/twolevel however, when someone tries to run MAGeT:

MAGeT.py: error: unrecognized arguments: --maget-masking-method minctracc --maget-lsq12-protocol /opt/quarantine/pydpiper/2.0.5/new-pydpiper-lsq12_2.csv --maget-nlin-protocol /opt/quarantine/pydpiper/2.0.5/new-pydpiper-minctracc-nlin.csv --maget-masking-nlin-protocol /opt/quarantine/pydpiper/2.0.5/new-pydpiper-minctracc-nlin.csv --maget-registration-method minctracc  

This is because the options are named differently for the MAGeT.py program than the MBM.py program

bcdarwin commented 6 years ago

Hmm ...

What I want (not entirely realized) is for pipelines to be composable but still configurable. If we're to use command-line settings for this rather than drive Pydpiper from a Python interactive shell it seems necessary to do something like this prefixing scheme (which is done semi-automatically when you create the different parsers). Note that even if this were allowed I don't see how it work work correctly (MAGeT would get your ANTs protocol, for example -- though maybe we the protocols themselves should determine the program used ...). We'd probably need some sort of syntax for you to walk the tree of pipeline components looking for MAGeT-things and adjusting them ... another reason why this should all be done from within a real programming language since I don't feel like making a set of conventions for specifying this in YAML ...

bcdarwin commented 6 years ago

Definitely open to suggestions though ...

gdevenyi commented 6 years ago

Hrm, I see what you mean.

For now, I've solved this by creating two possible config files and made the module print the warning to point at the other config if using MAGeT.py

bcdarwin commented 6 years ago

It occurs to me this could be done with PYDPIPER_ environment variables, which configargparse supposedly handles. I'll look into this.