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

pangolin --all-versions broken with 4.1 #467

Closed
HadrienG opened this issue Jun 30, 2022 · 15 comments
Closed

pangolin --all-versions broken with 4.1 #467

HadrienG opened this issue Jun 30, 2022 · 15 comments

Comments

@HadrienG
Copy link

I'm running pangolin as part of wf-artic (with docker, image: ontresearch/pangolin:4.0.5). It failed on this morning's samples:

pangolin 4.0.5:

epi2melabs@dad1d3221ca3:~$ pangolin -v
pangolin 4.0.5
epi2melabs@dad1d3221ca3:~$ pangolin --all-versions
pangolin: 4.0.5
pangolin-data: 1.3
constellations: v0.1.6
scorpio: 0.3.16

after running pangolin --update:

epi2melabs@dad1d3221ca3:~$ pangolin --update
Latest for pangolin is v4.1
pangolin updated to v4.1
Latest for pangolin-data is v1.11
pangolin-data updated to v1.11
Latest for constellations is v0.1.10
constellations updated to v0.1.10
Latest for scorpio is v0.3.17
scorpio updated to v0.3.17
epi2melabs@dad1d3221ca3:~$ pangolin -v
pangolin 4.1
epi2melabs@dad1d3221ca3:~$ pangolin --all-versions
pangolin: 4.1
pangolin-data: 1.11
constellations: v0.1.10
scorpio: 0.3.17
Traceback (most recent call last):
  File "/home/epi2melabs/conda/bin/pangolin", line 8, in <module>
    sys.exit(main())
  File "/home/epi2melabs/conda/lib/python3.8/site-packages/pangolin/command.py", line 156, in main
    print_versions_exit(config)
  File "/home/epi2melabs/conda/lib/python3.8/site-packages/pangolin/utils/initialising.py", line 226, in print_versions_exit
    print_conda_version(['usher', 'ucsc-fatovcf', 'gofasta', 'minimap2'])
  File "/home/epi2melabs/conda/lib/python3.8/site-packages/pangolin/utils/initialising.py", line 203, in print_conda_version
    result = subprocess.run(['conda', 'list', pkg],
  File "/home/epi2melabs/conda/lib/python3.8/subprocess.py", line 493, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/home/epi2melabs/conda/lib/python3.8/subprocess.py", line 858, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/home/epi2melabs/conda/lib/python3.8/subprocess.py", line 1704, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'conda'
@kapsakcj
Copy link

I'm also getting this error when running pangolin --all-versions with pangolin v4.1. Think it might be due to the recent PR #444 from @pvanheus ?

Here's the dockerfile that I'm using to build the docker image. Currently have pangolin --all-versions commented out https://github.com/StaPH-B/docker-builds/blob/4d90cc23f190b3a7d07e1baacffe7b25d211d24f/pangolin/4.1/Dockerfile

Basically it uses the environment.yml from Pangolin (with some slight tweaks to pin versions) and builds the environment with micromamba create -n pangolin -y -f /pangolin/environment.yml

Is this error being thrown because I don't have conda installed/available in my environment?

pangolin: 4.1
pangolin-data: 1.11
constellations: v0.1.10
scorpio: 0.3.17
pangolin-assignment: 1.11
Traceback (most recent call last):
  File "/opt/conda/envs/pangolin/bin/pangolin", line 8, in <module>
    sys.exit(main())
  File "/opt/conda/envs/pangolin/lib/python3.8/site-packages/pangolin/command.py", line 156, in main
    print_versions_exit(config)
  File "/opt/conda/envs/pangolin/lib/python3.8/site-packages/pangolin/utils/initialising.py", line 226, in print_versions_exit
    print_conda_version(['usher', 'ucsc-fatovcf', 'gofasta', 'minimap2'])
  File "/opt/conda/envs/pangolin/lib/python3.8/site-packages/pangolin/utils/initialising.py", line 203, in print_conda_version
    result = subprocess.run(['conda', 'list', pkg],
  File "/opt/conda/envs/pangolin/lib/python3.8/subprocess.py", line 493, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/opt/conda/envs/pangolin/lib/python3.8/subprocess.py", line 858, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/opt/conda/envs/pangolin/lib/python3.8/subprocess.py", line 1704, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'conda'

@aineniamh
Copy link
Member

aineniamh commented Jun 30, 2022

Thanks for flagging this @HadrienG and @kapsakcj. I think you might be right about the conda thing, I'll have a look now.

Although saying that, our github actions use mamba and seem to be working still.

@kapsakcj
Copy link

Although saying that, our github actions use mamba and seem to be working still.

If the GH actions environment includes conda alongside mamba (for example if conda is used to install mamba) then that may be why the GH actions workflow is running successfully. Hard to say though, I'm not too familiar with this GH actions workflow:

- uses: conda-incubator/setup-miniconda@v2

The name of it makes me think that miniconda is first installed, which comes with conda executable

@fmaguire
Copy link
Contributor

fmaguire commented Jun 30, 2022

Replacing this line

print_conda_version(['usher', 'ucsc-fatovcf', 'gofasta', 'minimap2'])
with the following:

for program in ['usher --version', 'gofasta --version', 'minimap2 --version',]:
    output = subprocess.run(program, shell=True, check=True,
                             stdout=subprocess.PIPE, encoding='utf-8')
    version = output.stdout.strip().split()[-1].strip('()v')
    print(f"{program.split()[0]} {version}")

would be a more robust/less conda dependent way to get the versions of all the dependencies apart from faToVcf (which doesn't have an integrated version flag). Might be worth asking ucsc folks about whether they can add that flag to their fork. This approach also means the correct tool version (i.e., the one pangolin will actually try to use) will be returned in situations where someone has messed up their conda path/config!

@aineniamh
Copy link
Member

Ah that makes sense so! I've got a potential fix here for your case @kapsakcj, but It assumes that people will either have conda or mamba installed though so not a fix for the original poster.

Ah, @fmaguire just seeing your message there- yeah this is what we had before. Maybe we should just revert to this style.

@kapsakcj
Copy link

Thanks @fmaguire and @aineniamh . I know usher --version exists, I requested that flag a while back 😄

gofasta also has that flag

$ gofasta --version
gofasta version 1.0.0

the tricky one might be ucsc-fatovcf

@aineniamh
Copy link
Member

aineniamh commented Jun 30, 2022

So that branch push there should do everything except the faToVcf now without relying on conda or mamba. I'll see if checks pass for that.

Edit: nope

@kapsakcj
Copy link

That commit (aca98ff) fixed it for me

@aineniamh
Copy link
Member

https://github.com/cov-lineages/pangolin/releases/tag/v4.1.1
This tag has the updated fix, but will update again when faToVcf can be included

@AngieHinrichs
Copy link
Member

Sorry folks -- not @pvanheus's fault but mine from #440. Every time something works for me in conda, I should know it's going to not work for somebody else -- or even for me, later, when I have made some change (installing mamba?), run pangolin --update and pangolin --all-versions and what do you know, suddenly my pangolin can't do conda list like I swear it could when I sent the PR.

TIL that conda is not in my path, but is instead a function:

conda activate pangolin

which conda
which: no conda in (/...:/cluster/home/angie/miniconda3/envs/pangolin/bin:...

command -V conda
conda is a function
conda () 
{ 
...

I see from the function that there is a $CONDA_EXE (in my environment it's /cluster/home/angie/miniconda3/bin/conda) which would probably work within pangolin -- in my installation, at the moment. I agree it's safer to avoid conda, which is disappointing since conda is our recommended installation method.

I will add a --version option to faToVcf, but it will take a few weeks to percolate through our software release cycle and then will require an update in bioconda. Running faToVcf --version with older versions will result in an error exit about the option not existing, but we can wrap a try/except around it...

@AngieHinrichs
Copy link
Member

AngieHinrichs commented Jun 30, 2022

Reading the code, the kent/src options processing lib module actually has a --version-ish behavior already, with a characteristically odd invocation:

faToVcf -verbose=2 -h

The first line of output includes the version:

### kent source version 426 ###

The -h makes it return with status 0 instead of 1 after printing out the usage.

@aineniamh
Copy link
Member

def print_faToVf_version():
    output = subprocess.run("faToVcf -verbose=2 -h 2>&1 | grep '#'", shell=True, check=True,
                                stdout=subprocess.PIPE, encoding='utf-8')
    version = output.stdout.split(' ')[-2]
    print(f"faToVcf: {version}")

I've added this in as a potential version print out?
Running it gives

pangolin: 4.1
pangolin-data: 1.11
constellations: v0.1.10
scorpio: 0.3.17
usher 0.5.4
gofasta 1.1.0
minimap2 2.24-r1122
faToVcf: 426

@AngieHinrichs
Copy link
Member

Excellent, thanks @aineniamh!

@kapsakcj
Copy link

kapsakcj commented Jul 1, 2022

Thanks for the quick fix @aineniamh and thank you for the explanation @AngieHinrichs ! Sorry @pvanheus , I didn't mean to point fingers. I clearly don't understand how to dig into python tracebacks

The 4.1.1 release has resolved this issue for me.

aineniamh added a commit that referenced this issue Jul 14, 2022
#467 potential fatovcf version print out
@aineniamh
Copy link
Member

Latest release has this issue fixed in it now v4.1.2.

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

No branches or pull requests

5 participants