Skip to content

Added optional parameter --nsubgenes#93

Merged
ggabernet merged 15 commits intoqbic-pipelines:devfrom
jonoave:add_feature_nsubgenes_dev
Mar 31, 2022
Merged

Added optional parameter --nsubgenes#93
ggabernet merged 15 commits intoqbic-pipelines:devfrom
jonoave:add_feature_nsubgenes_dev

Conversation

@jonoave
Copy link
Copy Markdown
Collaborator

@jonoave jonoave commented Jan 11, 2022

Many thanks to contributing to qbic-pipelines!

Please fill in the appropriate checklist below (delete whatever is not relevant). These are the most common things requested on pull requests (PRs).

PR checklist

  • This comment contains a description of changes (with reason)
    Added optional parameter --nsubgenes, due to potential errors with small dataset like miRNAs
    https://www.biostars.org/p/456209/

  • If you've fixed a bug or added code that should be tested, add tests!

  • Ensure the test suite passes (nextflow run . -profile test,docker).
    Tested with the following options, completed without errors:

nextflow run jonoave/rnadeseq -r add_feature_nsubgenes -profile test,cfc 
nextflow run jonoave/rnadeseq -r add_feature_nsubgenes -profile test,cfc --nsubgenes 500
  • Documentation in docs is updated
    Not updated.
  • CHANGELOG.md is updated
    Updated with line on v 1.4.1
  • README.md is updated
    Not updated.

Learn more about contributing: https://github.com/qbic-pipelines/rnadeseq/tree/master/.github/CONTRIBUTING.md

Comment thread CHANGELOG.md Outdated
Copy link
Copy Markdown
Collaborator

@ggabernet ggabernet left a comment

Choose a reason for hiding this comment

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

Hi @jonoave , thanks a lot for the helpful contribution. I just have a comment regarding the parameter name, which could be a bit more precise. Other than that it looks good!

Comment thread main.nf Outdated
Comment thread main.nf Outdated
Co-authored-by: Gisela Gabernet <gisela.gabernet@gmail.com>
@d4straub
Copy link
Copy Markdown
Contributor

I think the change that Gisela proposed needs to be also done in almost all other files (not in the R script though).

@jonoave
Copy link
Copy Markdown
Collaborator Author

jonoave commented Jan 19, 2022

I think the change that Gisela proposed needs to be also done in almost all other files (not in the R script though).

Thanks for the reminder Daniel, I've made the changes to the other files as well, and tested the rerun on cluster with
nextflow run jonoave/rnadeseq -r add_feature_nsubgenes_dev -profile test,cfc --vst_genes_number 500

The pipeline completed successfully

Copy link
Copy Markdown
Collaborator

@ggabernet ggabernet left a comment

Choose a reason for hiding this comment

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

Hi @jonoave ,

thanks for the changes, looks good to me now, I just have a minor comment. The failing tests are due to some updates that are needed in the CI workflows, etc. That can be fixed in a separate PR so don't worry about them...

Comment thread assets/RNAseq_report.Rmd Outdated
@ggabernet ggabernet self-requested a review February 22, 2022 08:12
Comment thread docs/usage.md Outdated
@ggabernet
Copy link
Copy Markdown
Collaborator

Great, thanks for your contribution @jonoave , once the tests pass we can merge!

WackerO and others added 6 commits March 31, 2022 08:25
Changed indentations, this should fix the Markdownlint error
Removed trailing whitespace, this should fix the EditorConfig error
Again removed trailing whitespace...
Trailing whitespace the third
@ggabernet ggabernet merged commit aca31e5 into qbic-pipelines:dev Mar 31, 2022
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.

4 participants