search

BIC-MNI / minc-tools

Basic minc-tools from former minc repository
Other
30 stars 25 forks source link

Argument parsing #24

Open andrewjanke opened 9 years ago

andrewjanke commented 9 years ago

mincreshape (and a few other tools) can be a bit picky about their input arguments, as an example:

$ mincreshape -dimrange zspace=0001,0 ...

Will fail as it doesn't understand 0001 as 1. This can be a pain when calling mincreshape and friends from within scripts. What's the consensus on what the behaviour should be for this?

rdvincent commented 9 years ago

Looking at the mincreshape code, it seems to do something reasonable here, which is pass the argument string to strtol(). My only concern is that it uses strtol() with base=0, so it interprets a leading '0' as starting an octal number. So 0001 should actually work, but 0008 will fail somewhat mysteriously.

The easy thing would be to disallow octal and hex here and hand an explicit base=10 to strtol(), as I seriously doubt anyone relies on passing octal values. I'm less sure about hex values, but they also seem pretty unlikely.

andrewjanke commented 9 years ago

I'm for it! (and am with you on the unlikely use of octal and hex).

Thanks for hunting this one down, it took me long enough to figure what was going wrong with my nipype script that was calling a perl script that was calling shell that was.... At that point I found the strange error was due to mincreshape I decided it was time to stop digging.

rdvincent commented 9 years ago

A quick glance shows that ParseArgv.c does something similar with its integer arguments.

Just to be safe, I might implement a quick solution that recognizes a leading 0x/0X as indicating hex, but treats everything else as decimal. I think that would be the most conservative fix - I can't imagine anyone is relying on passing octal arguments, but I can't completely rule out hex in some contexts.

andrewjanke commented 9 years ago

Works for me. The leading 0x is consistent with a number of other command line tools and scripting languages.

Thanks

rdvincent commented 9 years ago

OK, I checked in these changes in the develop branch. Check it out when you have a spare tic.

andrewjanke commented 9 years ago

Looks good to me.

Thanks

a

On 3 June 2015 at 00:09, Robert D Vincent notifications@github.com wrote:

OK, I checked in these changes in the develop branch. Check it out when you have a spare tic.

— Reply to this email directly or view it on GitHub https://github.com/BIC-MNI/minc-tools/issues/24#issuecomment-107967976.