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

Add support for the non-free netMHC tool family #73

Merged
merged 7 commits into from Jan 29, 2021
Merged

Add support for the non-free netMHC tool family #73

merged 7 commits into from Jan 29, 2021

Conversation

lkuchenb
Copy link
Collaborator

@lkuchenb lkuchenb commented Jan 25, 2021

PR checklist

Additions

This patch adds support for the non-free 3rd party tools NetMHC, NetMHCpan, NetMHCII and NetMHCIIpan. Since the tools' licenses do not allow for redistribution of the software, the user has to provide the software tarball which can be obtained from the upstream website.

  • To maintain reproducibility, the pipeline validates the checksum of the provided software tarballs
  • The patch provides a process that
    • Unpacks the software tarballs
    • Performs necessary modifications to the tools wrapper scripts as required by the installation instructions
    • Downloads the model data as required by installation instructions. Again, checksums are being checked.

Changes

  • epaa.py: Make tool name comparison case insensitive. Unfortunately, the method names aren't uniform w.r.t. upper / lower case letters among the netMHC family of tools.
  • main.nf: Convert params.tools to a groovy list before parsing. The previous .contains() calls were applied on strings, which worked fine for the previously used tools, but since e.g. netmhc is a substring of netmhcpan it would clash with the newly added tools

@lkuchenb lkuchenb force-pushed the add/netmhc branch 2 times, most recently from 396d9bf to f1137eb Compare January 26, 2021 11:41
@lkuchenb lkuchenb marked this pull request as ready for review January 26, 2021 11:48
@lkuchenb
Copy link
Collaborator Author

This is ready for review from my end. It's worth noting that the added CI will fail when the PR comes from a fork, i.e. only nf-core team members who can open a branch here can also run the CI successfully. Thus I guess it should stay optional. Since we discussed the non-free integration on Slack, maybe @nf-core/core could also cross check.

Copy link
Collaborator

@christopher-mohr christopher-mohr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just some minor things from my side. Thanks!
Let's wait for feedback from the nf-core core team regarding your question before merging.

conf/test_netmhcii.config Show resolved Hide resolved
nextflow_schema.json Outdated Show resolved Hide resolved
nextflow_schema.json Outdated Show resolved Hide resolved
nextflow_schema.json Outdated Show resolved Hide resolved
nextflow_schema.json Outdated Show resolved Hide resolved
nextflow_schema.json Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
lkuchenb and others added 2 commits January 27, 2021 14:59
Co-authored-by: Christopher Mohr <christopher.mohr@uni-tuebingen.de>
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.

None yet

2 participants