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

read_qc subworkflow #15

Merged
merged 13 commits into from
Sep 27, 2023
Merged

read_qc subworkflow #15

merged 13 commits into from
Sep 27, 2023

Conversation

chrisAta
Copy link
Member

Adding subworkflow that performs read QC using two modules:

  • fastp (imported from nf-core, modified slightly to remove parameters we don't need)
  • seqtk/seq (imported from nf-core)

@KateSakharova
Copy link
Contributor

General question. If you took a module from nf-core should it be under nf-core folder rather than ebi-metagenomics?

@chrisAta
Copy link
Member Author

General question. If you took a module from nf-core should it be under nf-core folder rather than ebi-metagenomics?

Good question... I think from what @mberacochea told me before you can't have modules from more than one 'nf-core repo' for a custom modules repo, so I think the nf-core tools would break if we tried. For example trying to install nf-core/fastp:

nf-core modules install fastp

                                          ,--./,-.
          ___     __   __   __   ___     /,-._.--~\
    |\ | |__  __ /  ` /  \ |__) |__         }  {
    | \| |       \__, \__/ |  \ |___     \`-._,-`-,
                                          `._,._,'

    nf-core/tools version 2.10.dev0 - https://nf-co.re


ERROR    You cannot install a fastp in a clone of nf-core/modules  

And I think some of the nf-core tools look at org_path in .nf-core.yml to know where to look for modules, and in this repo we have it set to ebi-metagenomics, so if we manually created an nf-core directory I don't think it would look there anyway

@KateSakharova
Copy link
Contributor

KateSakharova commented Sep 19, 2023

@chrisAta OK, I see.
In my vision we should definetely distinguish nf-core taken tools and our self-written. We need to mark it somehow or create a symlink maybe?
I'm just thinking it can be a case we bump into very BAD nf-core written module and we want to re-create it completely from our side (less likly but who knows...) what we should do?
Having nf-core subfolder can resolve it

Those are just are my thoughts for future. No need to do anything now :)

@chrisAta
Copy link
Member Author

@chrisAta OK, I see. In my vision we should definetely distinguish nf-core taken tools and our self-written. We need to mark it somehow or create a symlink maybe? I'm just thinking it can be a case we bump into very BAD nf-core written module and we want to re-create it completely from our side (less likly but who knows...) what we should do? Having nf-core subfolder can resolve it

Those are just are my thoughts for future. No need to do anything now :)

Yes I completely agree - would be nice to have a more obvious and defined way for making it clear we didn't build a module from scratch 😄 Something to discuss in near future

@mberacochea
Copy link
Member

@chrisAta OK, I see. In my vision we should definetely distinguish nf-core taken tools and our self-written. We need to mark it somehow or create a symlink maybe? I'm just thinking it can be a case we bump into very BAD nf-core written module and we want to re-create it completely from our side (less likly but who knows...) what we should do? Having nf-core subfolder can resolve it

Those are just are my thoughts for future. No need to do anything now :)

I agree, copying modules over is far from ideal but it's the best solution we have ATM. nf-core tools doesn't support subworkflows with modules from different repos (nf-core/tools#1927). The other option would be to symlink the modules, but that would create some issues down the road and will be tricky to maintain and deploy.

The best solution would be for subworkflows to support modules from nf-core and custom repos.

Copy link
Member

@mberacochea mberacochea left a comment

Choose a reason for hiding this comment

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

Great stuff @chrisAta .. I've added some minor comments.

@chrisAta
Copy link
Member Author

Great stuff @chrisAta .. I've added some minor comments.

All done! Thanks for the review 😄

…d into seqtk/seq. add unit tests for reads_qc subworkflow that test for just single-end, just paired-end, and a mix
Copy link
Member

@mberacochea mberacochea left a comment

Choose a reason for hiding this comment

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

One last bit.. the nextflow.config file for the subworkflow.

Copy link
Member

@mberacochea mberacochea left a comment

Choose a reason for hiding this comment

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

Great stuff :shipit:

@KateSakharova
Copy link
Contributor

Thank you Chris!

@chrisAta chrisAta merged commit aedfa69 into main Sep 27, 2023
@chrisAta chrisAta deleted the swf/qc branch September 27, 2023 10:00
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.

3 participants