BioContainers / containers

Bioinformatics containers
http://biocontainers.pro
Apache License 2.0
674 stars 246 forks source link

CADD 1.6.post1 container #547

Closed fa2k closed 9 months ago

fa2k commented 9 months ago

I recently submitted a container for CADD 1.6 in PR #546 and it was merged. At the time, I didn't understand that there's a difference between CADD-scripts 1.6 and CADD-scripts 1.6.post1 (had compared in github but did it wrong).

There are some really good improvements in CADD-scripts 1.6.post1 so I'd like to submit a container for this. This doesn't seem to be in bioconda, not in the second build of 1.6 cadd-scripts-1.6-hdfd78af_1 (Maybe a bug in the bioconda packaging).

This PR addds an additional version of cadd, for CADD-scripts v1.6.post1. In the "Commits" and "Files Changed" sections it also shows adding the 1.6 version that was merged. I'm using the same source branch for the new PR, but let me know if I should create a new one instead.

I think the original container hasn't gone into the registry yet - so if you prefer, I can delete cadd-scripts-with-envs 1.6. I think it's unlikely people will need to use that, over the post1 release.

biocontainers-bot commented 9 months ago

can't modify multiple containers in a same pull request

biocontainers-bot commented 9 months ago

No biotools label defined, please check if tool is not already defined in biotools (https://bio.tools) and add extra.identifiers.biotools label if it exists. If it is not defined, you can ignore this comment.

biocontainers-bot commented 9 months ago

No test-cmds.txt (test file) present, skipping tests

mboudet commented 9 months ago

Hmm, the previous container was built (https://hub.docker.com/r/biocontainers/cadd-scripts-with-envs), but does not appears in the search on biocontainers for some reasons. I'll look into it.

For cadd-scripts, might be good to look into why bioconda does not have the latest release.. In any case, this PR looks fine to me. Did you test it, feature-wise?

fa2k commented 9 months ago

Yes I also saw that the container was not in the registry.

I don't know anything about conda packaging, but will try to have a look at it.

I've tested annotation with the new image on Docker and Singularity. The reason I wanted 1.6.post1 was that in a pipeline, there may no variants in an input file (or intermediate file) and the previous cadd-scripts would crash in that case. The new version fixes this issue.

mboudet commented 9 months ago

Merging, thanks!

fa2k commented 8 months ago

@mboudet I hope you see this message even though the PR is closed. The builds failed for this change and for the small change later. https://github.com/BioContainers/containers/actions/runs/6979170066 https://github.com/BioContainers/containers/actions/runs/6982543489

The build for the original CADD 1.6 container did seem to succeed https://github.com/BioContainers/containers/pull/546/checks, but neither of the cadd-scripts-with-envs versions (1.6 and 1.6.post1) seem to make it to the registry.

What could be going wrong here, and is there any checks I can do?

mboudet commented 8 months ago

@fa2k The 'failed' CI when merging the README changes were expected (the CI expect a dockerfile change, and does not handle other changes well).

As far as I can see, both containers were built: https://hub.docker.com/r/biocontainers/cadd-scripts-with-envs/tags

If you are talking about the containers not being visible on https://biocontainers.pro/ , I believe there are some issues on the website. I asked the person in charge a few weeks back, but did not get an answer yet. (https://github.com/BioContainers/biocontainers-backend/issues/27).

fa2k commented 8 months ago

Thanks for the reply, that's very helpful! Indeed I can pull from Docker Hub. I was confused because I'm testing a netxtflow module for this (https://github.com/nf-core/modules/pull/4610), and it was trying to pull from quay.io by default, which gives an error unauthorized: access to the requested resource is not authorized.. (Then I got further confused by checking biocontainers.pro) The new image doesn't exist in quay.io - is that only for the conda-based packages? It's working well when I force it to use docker.io though.

mboudet commented 8 months ago

Yes, the quay.io 'biocontainers' should only contains images directly built from bioconda. (I agree that's not very clear).