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

updating the sourmash sketch module #2176

Merged
merged 15 commits into from
Oct 18, 2022

Conversation

taylorreiter
Copy link
Contributor

@taylorreiter taylorreiter commented Oct 7, 2022

This PR updates the sourmash sketch module.

In main.nf:

  • upgrades to the latest version of sourmash (4.5.0)
  • updates the params string to the standard set of parameters typically used by sourmash (including to sketch all of GenBank and the SRA). The test data should stay the same, and the way the param string is parameterized hasn't changed so can still be updated to the user if they wish to not follow this convention.

In meta.yml:

  • updates the doi to the correct citation (currently listed as mash, which is a different tool than sourmash)
  • updates the url to the correct sourmash github repo
  • updates the description from MinHash to FracMinHash
  • updates the types of data ingested to include protein sequences, transcriptomes, and fastq files

PR checklist

  • This comment contains a description of changes (with reason).
  • Follow the parameters requirements.
  • Add a resource label
  • Use BioConda and BioContainers if possible to fulfil software requirements.
  • Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
    • PROFILE=docker pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware
    • PROFILE=singularity pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware
    • PROFILE=conda pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware

I'm not sure what the checklist item about following parameter requirements refers to so I didn't check that box for now.

@taylorreiter taylorreiter changed the title [WIP] updating the sourmash sketch module updating the sourmash sketch module Oct 7, 2022
Copy link
Contributor

@Midnighter Midnighter left a comment

Choose a reason for hiding this comment

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

Update looks good to me. I have a slight doubt about the default arguments but everything else is 👍🏼

modules/nf-core/sourmash/sketch/main.nf Show resolved Hide resolved
modules/nf-core/sourmash/sketch/meta.yml Outdated Show resolved Hide resolved
modules/nf-core/sourmash/sketch/meta.yml Outdated Show resolved Hide resolved
Co-authored-by: Moritz E. Beber <midnighter@posteo.net>
Copy link
Contributor

@Midnighter Midnighter left a comment

Choose a reason for hiding this comment

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

Thanks for the update. I didn't know about the automatic test for versions.yml either. So just a little change and this is good to go.

taylorreiter and others added 3 commits October 12, 2022 09:10
Co-authored-by: nvnieuwk <101190534+nvnieuwk@users.noreply.github.com>
modules/nf-core/sourmash/sketch/main.nf Show resolved Hide resolved
modules/nf-core/sourmash/sketch/meta.yml Outdated Show resolved Hide resolved
taylorreiter and others added 3 commits October 13, 2022 07:54
Co-authored-by: James A. Fellows Yates <jfy133@gmail.com>
Co-authored-by: James A. Fellows Yates <jfy133@gmail.com>
@taylorreiter taylorreiter merged commit 80650e3 into nf-core:master Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants