Skip to content

Bf/quoted commands

Paul McCarthy requested to merge bf/quoted_commands into master

Hi @duncan, this MR addresses an issue in fsl_sub whereby it would not accept command files which contain quoted commands, e.g.:

"/dir/without/spaces/command"
"/dir with spaces/command"

Recent changes to bedpostx_gpu have caused it to generate a commands.txt file where the full path to xfibres_gpu is quoted, e.g.:

"/opt/fmrib/fsl/bin/xfibres_gpu" --data="/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/data_0" --mask="/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/nodif_brain_mask" -b "/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/bvals" -r "/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/bvecs" --forcedir --logdir="/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/diff_parts/data_part_0000" --nf=3 --fudge=1 --bi=1000 --nj=1250 --se=25 --model=2 --cnonlinear /home/fs0/paulmc/scratch/bedpostx_test/subj1 0 4 218893
"/opt/fmrib/fsl/bin/xfibres_gpu" --data="/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/data_1" --mask="/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/nodif_brain_mask" -b "/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/bvals" -r "/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/bvecs" --forcedir --logdir="/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/diff_parts/data_part_0001" --nf=3 --fudge=1 --bi=1000 --nj=1250 --se=25 --model=2 --cnonlinear /home/fs0/paulmc/scratch/bedpostx_test/subj1 1 4 218893
"/opt/fmrib/fsl/bin/xfibres_gpu" --data="/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/data_2" --mask="/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/nodif_brain_mask" -b "/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/bvals" -r "/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/bvecs" --forcedir --logdir="/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/diff_parts/data_part_0002" --nf=3 --fudge=1 --bi=1000 --nj=1250 --se=25 --model=2 --cnonlinear /home/fs0/paulmc/scratch/bedpostx_test/subj1 2 4 218893
"/opt/fmrib/fsl/bin/xfibres_gpu" --data="/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/data_3" --mask="/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/nodif_brain_mask" -b "/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/bvals" -r "/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/bvecs" --forcedir --logdir="/home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/diff_parts/data_part_0003" --nf=3 --fudge=1 --bi=1000 --nj=1250 --se=25 --model=2 --cnonlinear /home/fs0/paulmc/scratch/bedpostx_test/subj1 3 4 218893

The old fsl_sub would accept this syntax, whereas the new fsl_sub was raising an error, e.g.:

Error submitting job - Array task definition file fault: Cannot find script/binary "/opt/fmrib/fsl/bin/xfibres_gpu" on line 1 of /home/fs0/paulmc/scratch/bedpostx_test/subj1.bedpostX/commands.txt

I've also added Gitlab CI configuration so that unit tests are run on every push to this repository.

Finally, I was seing some failures when running the unit tests locally - in fsl_sub/tests/test_fsl_sub_shell.py you are using indirect variable references, which were causing tests to fail on my machine. There seems to be some variation in the behaviour of different shells with respect to the use of indirect variable references - these tests have passed for me in certain docker environments. On my laptop however, if I set a variable to an empty string, and then try to de_reference it, I get a bad subsitution error, e.g.:

$ var=""
$ echo ${!var} 
bash: : bad substitution  

From what I can gather, taking an indirect reference of an empty variable should result in an error (see e.g. https://lists.gnu.org/archive/html/bug-bash/2016-11/msg00165.html), so I suspect that their use in the test_fsl_sub_shell.py is invalid, and have therefore removed them.

Merge request reports