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

modules addition in basic training #2133

Merged

Conversation

FriederikeHanssen
Copy link
Contributor

No description provided.

Copy link

netlify bot commented Dec 1, 2023

Deploy Preview for nf-core ready!

Name Link
🔨 Latest commit 458b651
🔍 Latest deploy log https://app.netlify.com/sites/nf-core/deploys/65f9b092bcb9c80008ea0e58
😎 Deploy Preview https://deploy-preview-2133--nf-core.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@FranBonath FranBonath left a comment

Choose a reason for hiding this comment

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

very good start!

src/content/docs/contributing/nf_core_basic_training.md Outdated Show resolved Hide resolved
src/content/docs/contributing/nf_core_basic_training.md Outdated Show resolved Hide resolved
src/content/docs/contributing/nf_core_basic_training.md Outdated Show resolved Hide resolved
src/content/docs/contributing/nf_core_basic_training.md Outdated Show resolved Hide resolved
fasta.view()

SALMON_INDEX(
fasta.map{it -> [id:it.getName(), it]}
Copy link
Member

Choose a reason for hiding this comment

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

Salmon index requires two input Channels: genome_fasta and transcript_fasta, both as a path. If I understand this right, you are only giving one input, which is not a path? Also, when I tried running this, it won't accept the "id:" label for some reason, I get an syntax error which vanishes when I remove the label.

SALMON_INDEX()
```

Now we are still missing an input for our module. In order to build an index, we require the reference fasta. Luckily, the template pipeline has this already all configured, and we can access it by just using `params.fasta` and `view` it to insppect the channel content. (We will see later how to add more input files.)
Copy link
Member

Choose a reason for hiding this comment

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

Could we add a section where we show how to determine which input channels are required?


This makes the module now available in the workflow script and it can be called with the right input data.

<!-- TODO/TO DISCUSS here the user now needs to know about how to get their fasta. We could do this here or add a new point for this above -->
Copy link
Member

Choose a reason for hiding this comment

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

My logic would be having separate points:

  • install the module
  • add the inclusion statement and the process
  • identify which inputs are required, add (if necessary) a new param, add the input channels with optional channel manipulation
  • check the output channels to be used in another process

@FranBonath
Copy link
Member

@FriederikeHanssen do you think we can merge this PR until the retreat? Independent of how far we come, because then we could show one preview page instead of several PRs.

@FranBonath
Copy link
Member

@nf-core-bot fix linting please :)

@FranBonath FranBonath marked this pull request as ready for review March 19, 2024 15:54
@FranBonath FranBonath merged commit b99ab35 into nf-core:nf_core_basic_training Mar 19, 2024
6 checks passed
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