-
Notifications
You must be signed in to change notification settings - Fork 26
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
Release 2.2.0 #136
Release 2.2.0 #136
Conversation
Template update 2.12
Co-authored-by: Júlia Mir Pedrol <mirp.julia@gmail.com>
Crisprcleanr different input (csv and defined library)
…into bowtie_bis
Co-authored-by: Júlia Mir Pedrol <mirp.julia@gmail.com>
Co-authored-by: Júlia Mir Pedrol <mirp.julia@gmail.com>
Add bowtie2 in screening
…to mageckFlute
add empty reads_counts
fix prlotter, pattern doesn't exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 🚀 I only was wondering about your full tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! I have left a few comments related to aesthetics. Feel free to argue (not with the alignment one though).
meta.id = "${meta.treatment}_vs_${meta.reference}" | ||
|
||
""" | ||
#!/usr/bin/env Rscript |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider putting this in a separate file in bin
, a la carte scripts are technically allowed, but not really pretty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @itrujnara i'm looking into this now, and i was wondering if it's not a bit less readable since we have variables? Would you just take the whole R script as it is right now and put it in matricecreations.R
in bin for instance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say making it a template under templates
is the best in this case. This way, you can continue using the same nextflow variable interpolation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i just find it's less readable to understand what the module is doing? Is there a specific reason why we don't want any code in the script part?
def prefix = task.ext.prefix ?: "${meta.treatment}_vs_${meta.reference}" | ||
|
||
""" | ||
#!/usr/bin/env Rscript |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
avoid converting T nucleotides to TRUE
Update test_screening_paired.config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome job! Just drop some suggestions but nothing serious 😄
@@ -3,11 +3,34 @@ | |||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) | |||
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | |||
|
|||
## [v2.2.0 - Romarin Curie] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a reminder that it needs to be updated with the release date 😄
@@ -209,8 +209,8 @@ def report_bagel_version(): | |||
""" | |||
print( | |||
"Bayesian Analysis of Gene EssentiaLity (BAGEL) suite:\n" | |||
"Version: {VERSION}\n" | |||
"Build: {BUILD}".format(VERSION=VERSION, BUILD=BUILD) | |||
f"Version: {VERSION}\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if you have already tried, but won't be nice to try to add it to bioconda and thus the corresponding modules could be added to the modules repository. Found this issue were someone tried in the past but seems not to be there yet. Maybe not for this release but for a future one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey we opened an issue for it :
#155
Update mageck modules with nf-test
add suggestions from code review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome job! Thanks a lot! 🚀
PR checklist
nf-core lint
).nextflow run . -profile test,docker --outdir <OUTDIR>
).docs/usage.md
is updated.docs/output.md
is updated.CHANGELOG.md
is updated.README.md
is updated (including new tool citations and authors/contributors).