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

bug: CI is not using the specified Python version #108

Closed
fgypas opened this issue Mar 9, 2023 · 3 comments · Fixed by #124
Closed

bug: CI is not using the specified Python version #108

fgypas opened this issue Mar 9, 2023 · 3 comments · Fixed by #124
Assignees
Labels
bug Something isn't working

Comments

@fgypas
Copy link
Member

fgypas commented Mar 9, 2023

Describe the bug
The github action CI (.github/workflows/ci.yml) is not using the specified Python version.

To Reproduce
Although we specify python 3.7.4 in the field python-version https://github.com/zavolanlab/zarp/blob/dev/.github/workflows/ci.yml#L27 it switches to the default Python 3.10

Expected behavior
Use 3.7.4

Screenshots

We specify that properly
Screenshot 2023-03-10 at 00 20 30

But uses 3.10
Screenshot 2023-03-10 at 00 21 39

Log here: https://github.com/zavolanlab/zarp/actions/runs/4378771574/jobs/7663932202

@uniqueg
Copy link
Member

uniqueg commented Mar 10, 2023

Weird! ...or perhaps not!

The latest Python 3.7 release is 3.7.16 (from last December). Python 3.7.4 was released in mid-2019. There are numerous security vulnerabilities in that release that have been patched since. This could possibly be at least part of the reason why Python 3.7.4 seems to have disappeared from conda-forge:

image

This is likely the reason for the bug (although it'd be nice if the action would report that error). At least on the command line, when I try:

conda install -c conda-forge python=3.7.4

I get

[...]

Your installed version is: not available

So what to do?

As patch releases are not only backwards-compatible, but also shouldn't change the API, I don't see any reason why we shouldn't use a recent 3.7 release instead.

Except: We shouldn't be using Python 3.7 at all!

End of support for Python 3.7 is in a bit over 3 months:

image

Why not put in a bit of work to upgrade to a more recent minor release, like for example 3.10 (3.10.10)? 🙃

Possibly some extra work. Or perhaps not. It may be that after upgrading Mamba (they're on 1.3.1 now, plenty of releases since 0.16.0!) everything will work just fine!

And while we are at it, we could also try to upgrade:

  • Snakemake (7.24.1 vs 7.2.1; we also wouldn't need to specify tabulate anymore, as that bug was fixed in 7.15.2)
  • Singularity (3.8.7 vs 3.5.2)

With bedtools and biopython I'd be more careful. However, the versions we are using aren't all that old anyway, I think. So unless upgrading Python forces us to upgrade these as well, I would probably leave them as they are.

Finally, we currently do not specify a version for snakefmt. However, from #109 we know that 0.8.0 is buggy. So perhaps we could also put snakefmt>=0.8.1 .

@uniqueg
Copy link
Member

uniqueg commented Mar 10, 2023

Following @fgypas in #110, I think not pinning versions may be a good fit for us in all cases where it's not really necessary. In fact, I would probably only pin the Python version to a minor release (e.g., 3.10, instead of 3.10.10), as well as bedtools and biopython (i.e., leave them as they are unless we must upgrade with a newer Python).

For all other dependencies, _including Snakemake and Singularity, I would not specify a version at all, unless it turns out we really have to. Sooner or later we may start realizing that our tests fail because of some breaking changes in, e.g., Snakemake. But by being to conservative, we also need to do maintenance (like addressing this bug), and by not pinning versions, at least resolving dependencies becomes less of an issue. Plus we (and our users) will be using more secure software.

Given that the dependencies for individual rules are pinned in the workflow itself, I also don't think that reproducibility is much of a concern here (except possibly for bedtools and biopython).

Note that this is also the strategy that the Snakemake Workflow Catalog is using.

@mkatsanto
Copy link
Collaborator

Check if any rules use bare python envs and ensure there is compatibility with any updates.

balajtimate added a commit that referenced this issue Mar 22, 2023
Updated python, snakemake, bedtools, biopython versions.
balajtimate added a commit that referenced this issue Mar 22, 2023
balajtimate added a commit that referenced this issue Mar 22, 2023
balajtimate added a commit that referenced this issue Mar 23, 2023
balajtimate added a commit that referenced this issue Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants