Skip to content

ENH: Add _cmd_prefix class variable to CommandLine #2379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 22, 2018

Conversation

effigies
Copy link
Member

This PR adds a _cmd_prefix which is a string that is prepended (with no space) to the command. This will permit users to add installation-specific prefixes, whether they be installation directories that are not in the path or idiosyncratic names that have been chosen by a package manager.

The idea is that users could do something like:

import nipype
nipype.interfaces.ants.AntsCommand._cmd_prefix = '/opt/ants/'

or

import nipype
nipype.interfaces.fsl.FSLCommand._cmd_prefix = 'fsl_'

Fixes #1415.

cc @satra @TheChymera

@effigies effigies added this to the 1.0 milestone Jan 18, 2018
@effigies
Copy link
Member Author

@satra If you want to have a look at this to make sure it's what you were thinking, that'd be great.

@satra
Copy link
Member

satra commented Jan 19, 2018

in general this looks good.

@effigies
Copy link
Member Author

@satra Testing found what I think you were thinking of, which was a which function. Can you have a quick look at this latest commit?

@@ -987,14 +988,26 @@ def _run_interface(self, runtime, correct_return_codes=(0, )):

# which $cmd
executable_name = self.cmd.split()[0]
cmd_path = which(executable_name, env=runtime.environ)

prefix_parts = self._cmd_prefix.split()
Copy link
Member

@satra satra Jan 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah - so this is where spaces in the prefix would break this. also in the line above (for executable name), it assumes that self.cmd is not a full path with spaces.

perhaps this may help:

if os.path.isdir(self._cmd_prefix):
    executable_name = self._cmd_prefix + self.cmd 
elif os.path.isfile(self._cmd_prefix):
    executable_name = self._cmd_prefix
else:
    prefix = ''
    for part in self._cmd_prefix.split():
        prefix = prefix + part
        if os.path.isfile(prefix):
            executable_name = prefix
            break
    else:
        raise ValueError('{} does not contain a valid path'.format(self._cmd_prefix))
cmd_path = which(executable_name, env=runtime.environ)

@effigies
Copy link
Member Author

@satra I opted to go with shlex.split(prefix + cmd), which should be more reliable than any parsing strategy we choose.

@effigies
Copy link
Member Author

This is ready, as far as I'm concerned.

@effigies
Copy link
Member Author

@satra This is the only thing left to review for 1.0.

@satra satra merged commit 0ba35f1 into nipy:master Jan 22, 2018
@effigies effigies deleted the enh/command_prefix branch January 22, 2018 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prefixed names for interface functions
2 participants