cov-lineages / pangolin

Software package for assigning SARS-CoV-2 genome sequences to global lineages.
GNU General Public License v3.0
427 stars 107 forks source link

Python version conflict with Snakemake #525

Closed dskola closed 1 year ago

dskola commented 1 year ago

Pangolin 4.3 requires python >= 3.7 but the latest version of Snakemake requires python >= 3.9 (see https://github.com/snakemake/snakemake/commit/e9f6731858ba851b689236d89c26e739852fa3d6#diff-fa602a8a75dc9dcc92261bac5f533c2a85e34fcceaff63b3a3a81d9acde2fc52)

So, for a environment with, say, python 3.8, pangolin installation proceeds normally, but at runtime Snakemake throws a fatal error regarding the inadequate python version.

I recommend either:

  1. Ensuring that the environment.yml file specifies the minimum required python across all dependencies (hard to determine)
  2. Pointing to a fixed version of snakemake in environment.yml to avoid future surprises introduced by dependency updates

PR with both proposed changes here: #524

wm75 commented 1 year ago

There is an ongoing discussion about this over at bioconda where we experienced the same issue with the latest pangolin recipe update: https://github.com/bioconda/bioconda-recipes/pull/42145#issuecomment-1648014404

For pangolin, I decided to restrict snakemake to <=7.30.1 until the next release to avoid introducing lots of dependency changes with just a new build of the bioconda package. The discussion about handling bioconda's snakemake-minimal package is still open (see https://github.com/bioconda/bioconda-recipes/issues/42160).

Maybe you'd want to restrict snakemake in the environment file here to <=7.30.1 as well for now to give people more freedom with respect to the Python version they want to use? The restriction could be removed, for example, when Python 3.8 reaches end of life next year (https://devguide.python.org/versions/) or when a new version of pangolin would really benefit from removing it?

corneliusroemer commented 1 year ago

I think the PR #526 should be a robust solution

corneliusroemer commented 1 year ago

This has been resolved in #524 and with release 4.3.1