Skip to content
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

Prefixed names for interface functions #1415

Closed
TheChymera opened this issue Mar 30, 2016 · 14 comments · Fixed by #2379
Closed

Prefixed names for interface functions #1415

TheChymera opened this issue Mar 30, 2016 · 14 comments · Fixed by #2379

Comments

@TheChymera
Copy link
Collaborator

At least one of the many functions nipype has interfaces for - FSL's cluster - collides with another function from another package that shares its name. To make matters worse, this other package (graphviz) is a direct dependency of FSL (for the full description of the conflict see this issue).

One of the most intuitive ways to fix this is by adding a prefix to the function which is called by fewer other packages. In this case that would be FSL's cluster. I have actually noticed that many sysadmins consistently add a prefix to all functions from FSL or AFNI.

So, should I as a package maintainer decide to prefix FSL's cluster as fsl_cluster on Gentoo - how could I make sure nipype is aware that it should call that when one invokes the Cluster() interface?

@satra
Copy link
Member

satra commented Mar 30, 2016

@TheChymera - the only way i can see this work is to add a command-prefix option to base classes in nipype. by default this is None, but can be specific to such things.

@mwaskom
Copy link
Member

mwaskom commented Mar 30, 2016

Is there a reason fsl interfaces just make a system call with the binary name, rather than with the full path to it (using $FSLDIR, like Feat does)?

@satra
Copy link
Member

satra commented Mar 30, 2016

no particular reason other than we expected things to be available on the command line, and kept it simple.

now that we are supporting more packages, it may make sense to think a bit about extending how underlying interfaces are supported. however the general package management, dependency, update, and composition issue is not something nipype should tackle (at least in the near future).

@TheChymera
Copy link
Collaborator Author

I think it is a sane assumption that the tools should be available from the command line. However, I don't see how we can make any one assumption about the naming of the functions.

  • Some package maintainers might opt to resolve conflicts by adding e.g fsl as either a prefix or suffix to all the functions of the package (which leads to ridiculous names since fsl already prefixes some of its functions with fsl)
  • Some package maintainers would opt to prefix only those functions which are not already prefixed
  • Some other maintainers (to which I will likely count myself) will opt to tweak the name of only those functions for which there is a known collision

The only way I can see all these cases being addressed, is if you can pass a command modifier to each Interface. The drawback of this being that the user will be expected to know how his package maintainer handles conflicts. Neurodebian might handle stuff diferently from NeuroGentoo, and many people install stuff independently of their package manager anyway...

Alternatively we could use $FSLDIR, but I think this uses a quirk of FSL in particular to eschew the problem. The fix won't work should we come across any AFNI conflicts - or does AFNI have something comparable? Normally you would expect all binaries installed for the entire system to live in peace and harmony in /usr/bin.

@TheChymera
Copy link
Collaborator Author

It seems the best way for a package manager to deal with FSL's myriad of functions and potential naming conflicts is to install all the executables in some $FSLDIR, e.g. /usr/share/fsl and place symlinks (referrably with some prefix, e.g. fsl_) to them under /usr/bin.

This is the way it is currently done in both NeuroGentoo and NeuroDebian.

I think the best way for nipype to be compatible with the only two larger repositories for neuroscientific software management is to call FSL functions via their ${FSLDIR}/bin/* path.

@satra @mwaskom can I enable this by simply adding a few lines to the FSLCommand() class where it is originally defined? I am not that well versed with classes, but it would be cool if I could just tell it to take th evalue of _cmd it gets when the instance is created, and then set _cmd to something based on that...

@djarecka
Copy link
Collaborator

@TheChymera , @satra , @mwaskom - are you still planning to modify the fsl cmd?

@TheChymera
Copy link
Collaborator Author

@djarecka no, Currently Gentoo is offering FSL up to 5.0.9 (I'll be including 5.0.10 soon), and on all versions we provide the executables under their original names, unless the respective executable is documented to conflict with another package; in this case we prepend fsl_. Currently this is only the case for cluster, which under Gentoo is usable as fsl_cluster.

We have chosen this approach for 2 reasons:

  • It's unwarranted to mess up the entire executable names for a whole package just because upstream wasn't careful and there's one conflict. We should only fix what's broken, and ideally those namespace collisions should be fixed by upstream (in which case all executables can be installed under their original names).
  • Applications like nipype expect to find the commands under certain names. Rather than developing a new standard for “how to change the entire FSL executable namespace” (something which is best handled not by us, but by FSL) or hacking the nipype install instructions to change all FSL command names, we again change only the minimum that is needed.

@djarecka
Copy link
Collaborator

@TheChymera - thanks for your answer. But if you changed in Gentoo after you changed cluster to `fsl_cluster, this example should fail, am I right? or did you add something to Nipype to know about this change?

@TheChymera
Copy link
Collaborator Author

@djarecka curently this example would fail. The right thing to do at this point would be:

  1. badger FSL upstream to fix the naming conflict,
  2. if it becomes apparent that that is unlikely to happen any time soon, or if this issue is reported as priority problem by users, patch the nipype ebuild.

Would you like to help with 1. ? :)

@djarecka
Copy link
Collaborator

djarecka commented Jan 3, 2018

@TheChymera - can try :)

@satra
Copy link
Member

satra commented Jan 3, 2018

@TheChymera , @djarecka - fsl is not going to change the name anytime soon. conflicts exist for many packages, and will continue to. the idea of namespaces on command lines effectively moved binaries from /usr/bin/ to /usr/share/package/bin/.

for the current purpose i think FSL should prefix every command with $FSLDIR/bin/

@satra
Copy link
Member

satra commented Jan 3, 2018

i meant in FSL's nipype interfaces we should use $FSLDIR/bin/ as a prefix to all FSL commands as suggested by @mwaskom

@mwaskom
Copy link
Member

mwaskom commented Jan 3, 2018

We could make this general by adding an input called bin_path (or similar) to the CommandLine interface object and then have the FSLCommand object set it to $FSLDIR. That would solve the specific problem and also make it easier for people with idiosyncratic name conflicts to specify which binary they want (potentially at the cost of reproducibility across systems...).

@TheChymera
Copy link
Collaborator Author

@satra

conflicts exist for many packages, and will continue to

Are you aware of any other conflicts with FSL? Maybe we should make a list to know what exactly is broken (if it actually is more than just cluster), so that we can actually report this.

fsl is not going to change the name anytime soon.

Have they stated as much?

the idea of namespaces on command lines effectively moved binaries from /usr/bin/ to /usr/share/package/bin/

This is a bad design path to go down. Giving each package its own tree encourages disregard for and general ignorance of standards, and perhaps worst of all, it encourages bundling. With the exception of serious fringe cases, modern systems can work perfectly without balkanizing their executable namespace:

chymera@zenbookhost ~ $ ls /usr/share/*/bin -dlah
lrwxrwxrwx 1 root root   23 13. Mär 2015  /usr/share/ant/bin -> /usr/share/ant-core/bin
drwxr-xr-x 2 root root 4.0K 13. Mär 2015  /usr/share/ant-core/bin
lrwxrwxrwx 1 root root    8 11. Okt 00:16 /usr/share/fsl/bin -> /usr/bin

make this general by adding an input called bin_path (or similar) to the CommandLine interface object and then have the FSLCommand object set it to $FSLDIR.

I know I agreed with this in the past, but in effect, this means that instead of addressing this specific error like what it is, an error, we would generalize it, and make it comfortable for other packages (and even better, users) to make more of the same error. Given the choice of making FSL cleaner or nipype messier, I think the first is preferable :)

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 a pull request may close this issue.

4 participants