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

Issue 106 #114

Merged
merged 8 commits into from
Nov 29, 2024
Merged

Issue 106 #114

merged 8 commits into from
Nov 29, 2024

Conversation

nschcolnicov
Copy link

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • If necessary, also make a PR on the nf-core/nanostring branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

Copy link

github-actions bot commented Nov 26, 2024

nf-core pipelines lint overall result: Passed ✅

Posted for pipeline commit e3531a4

+| ✅ 200 tests passed       |+

✅ Tests passed:

Run details

  • nf-core/tools version 3.0.2
  • Run at 2024-11-28 19:33:02

@nschcolnicov nschcolnicov marked this pull request as ready for review November 28, 2024 16:19
@nschcolnicov
Copy link
Author

@apeltzer @atrigila We wont be migrating these modules to nf-core for now, as I was told they weren't good candidates since they aren't based on a tool but on a single script which is only hosted in the pipeline.
I would still like to move forward with this PR, keeping them all local, so we still benefit of the work I did standardizing them and adding all of the necessary files and tests.

@@ -21,7 +21,7 @@ process COMPUTE_GENE_SCORES {
def args = task.ext.args ?: ''

"""
compute_gene_scores.R $geneset_yaml $counts $args
compute_gene_scores.R $gene_score_yaml $normalized_counts $args

Choose a reason for hiding this comment

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

Something like this can make it easier to read in the log :)

Suggested change
compute_gene_scores.R $gene_score_yaml $normalized_counts $args
compute_gene_scores.R \\
$gene_score_yaml \\
$args \\
$normalized_counts

modules/local/compute_gene_scores/main.nf Show resolved Hide resolved
@@ -20,7 +19,7 @@ process CREATE_GENE_HEATMAP {
def args = task.ext.args ?: ''

"""
compute_gene_heatmap.R $annotated_counts $counts $heatmap_genes_to_filter $args
compute_gene_heatmap.R $annotated_endo_data $normalized_counts $heatmap_genes_to_filter $args

Choose a reason for hiding this comment

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

Something like this can make it easier to read in the log :)

Suggested change
compute_gene_heatmap.R $annotated_endo_data $normalized_counts $heatmap_genes_to_filter $args
compute_gene_heatmap.R \\
$annotated_endo_data \\
$normalized_counts \\
$args \\
$heatmap_genes_to_filter

//
// MODULE: Compute gene-count heatmap for MultiQC report based on annotated (ENDO) counts
//
if(!params.skip_heatmap){

Choose a reason for hiding this comment

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

I think we can't have params inside subworkflows now. They should be only in the main workflow. There was a guideline somewhere written by Ben but I can't find it now.

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense, if this was an nf-core subworkflow this would be an issue. I don't think we will be migrating this subworkflow anytime soon, but since I already formatted it all, Ill do this too.

@nschcolnicov
Copy link
Author

@atrigila all set 😄

@nschcolnicov nschcolnicov merged commit 154a99e into dev Nov 29, 2024
10 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.

2 participants