Path hard written in python wrapper script

[Juke34]

Would it be possible to update the ipk.py wrapper script to avoid hard written path related to the ipk.py script location and use what is available in the PATH by default?

Instead of

    current_dir = os.path.dirname(os.path.realpath(__file__))

    # run IPK
    if states == 'nucl':
        if keep_positions:
            raise RuntimeError("--keep-positions is not supported for DNA.")
        else:
            bin = f"{current_dir}/bin/ipk/ipk-dna"
    else:
        if keep_positions:
            bin = f"{current_dir}/bin/ipk/ipk-aa-pos"
        else:
            bin = f"{current_dir}/bin/ipk/ipk-aa"

Get something like that:

@click.option('--bin_location',
              type=click.Path(exists=True),
              required=False, default='',
              help="""Location of the executables...""")

------------
    # by default bin_location should be an empty string...
    if bin_location:
       bin_location = os.path.join(bin_location, '') # add the trailing slash if it's not already there

    # run IPK
    if states == 'nucl':
        if keep_positions:
            raise RuntimeError("--keep-positions is not supported for DNA.")
        else:
            bin = f"{bin_location}ipk-dna"
    else:
        if keep_positions:
            bin = f"{bin_location}pk-aa-pos"
        else:
            bin = f"{bin_location}pk-aa"

By default user should put the tool in /usr/bin, so would be nice to tell in the installation instruction to use cmake with -B /usr/bin and if user use another location, provide an option in ipk.py to allow the user to set the location where the executable are located.

[nromashchenko]

Hi!

Would it be possible to update the ipk.py wrapper script to avoid hard written path related to the ipk.py script location and use what is available in the PATH by default?

Sure. But it will take some time before we make the next release, we're busy in some other branches. And if you already have the code, I invite you to fork and pull request this change if you would like to contribute :)

thanks

[blinard-BIOINFO]

Hi @nromashchenko @Juke34

Juke is proposing this following my conda recipe update of this morning. I actually already apply a patch to the IPK sources to avoid the harcoded path, and to make it compatible with $PREFIX in the conda build environment (@nikolai, this is a different variable than PREFIX related to C++ compilation, it is the prefix in bioconda-utils environement, where compilation and package build is done)

epik.py is intended as the only entry point to call the compiled binaries. So making this option is probably not adapted ? In the recipe I made sure that that calling epik.py which target the good binaries (which should not be called directly).

[nromashchenko]

By default user should put the tool in /usr/bin, so would be nice to tell in the installation instruction to use cmake with -B /usr/bin and if user use another location, provide an option in ipk.py to allow the user to set the location where the executable are located.

Okay another note is that it's a good idea to install everything with

cmake --install . --prefix DIRECTORY
export PATH=DIRECTORY/bin:$PATH

then, the python wrapper will be installed within binaries to the directory of your choice (e.g. /usr/local/bin or such). And normally you shouldn't think about how the wrapper finds the binary (it will find it).

once we're done with the conda package, this issue will become irrelevant

[Juke34]

Yes I started to look into this when I have seen your patch @blinard-BIOINFO . But with conda there are many ways to cheat so I'm not worry for the conda installation (We might have copy the ipk build directory into the bin where is located ipk.py, it would have worked ^^). I was more thinking about standalone installation, as IPK executable location have to stay synchronized with the ipk.py wrapper location, it might be source of error. Using the cmake --prefix as you mention @nromashchenko seems a good solution. Anyway it was just a detail. Thank you for your quick feedbacks