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

Update container image to include RTGtools #6639

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Jorisvansteenbrugge
Copy link

PR checklist

Closes #6637

The container image assigned to happy/happy did not include RTG tools which is sometimes necessary. I updated the container image to a working version.

  • This comment contains a description of changes (with reason).

  • Use BioConda and BioContainers if possible to fulfil software requirements.

    • I build a seqera wave container, but I'm not sure if that is desirable. Reviewer, please let me know if not.

@adamrtalbot
Copy link
Contributor

Although RTG tools is recommended, it's not necessary.

Note, you can change the existing container using configuration:

process {
    withName: HAPPY_HAPPY {
        container = 'community.wave.seqera.io/library/hap.py_rtg-tools:2ebb433f3ce976d3'
    }
}

@Jorisvansteenbrugge
Copy link
Author

Although RTG tools is recommended, it's not necessary.

Note, you can change the existing container using configuration:

process {
    withName: HAPPY_HAPPY {
        container = 'community.wave.seqera.io/library/hap.py_rtg-tools:2ebb433f3ce976d3'
    }
}

@adamrtalbot Thank you for your suggestion. I am aware that it is possible to configure the process to run with a different container image. However, as mentioned in issue #6637 it is currently not possible to use the vcfeval engine in hap.py with this module. Out of curiosity, wouldn't it make sense to provide a more complete container image by default?

@nvnieuwk
Copy link
Contributor

Is vcfeval still not working with the seqera container?

@adamrtalbot
Copy link
Contributor

@adamrtalbot Thank you for your suggestion. I am aware that it is possible to configure the process to run with a different container image. However, as mentioned in issue #6637 it is currently not possible to use the vcfeval engine in hap.py with this module. Out of curiosity, wouldn't it make sense to provide a more complete container image by default?

If you swap the container, you can enable vcfeval by adding the arguments, e.g. ext.args = '--engine=vcfeval'.

Images should be as small as possible for efficiency and portability. By including rtgeval we introduce complexity that will cause issues for a user.

Furthermore:

  • the environment.yml isn't updated
  • no input for the rtgtools cache is provided

Update the tool to fully work with rtgeval and add a test and we will have a fully working example.

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

Successfully merging this pull request may close these issues.

RTGtools missing in modules happy/happy
4 participants