-
Notifications
You must be signed in to change notification settings - Fork 21
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.3 #233
Release 2.3 #233
Conversation
… have no significant results
…way analysis adjustable
… GO:CC to speed it up
Heatmap error, datasources, shorten filenames
Update RNAseq_report.Rmd for DE information
Fix new multiqc
Version changes for release 2.3
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.
LGTM (within my current knowledge of rnadeseq and R)! Still I had some minor comments 👍
Co-authored-by: Till E. <[email protected]>
Add distance info for heatmaps, add heatmap tables, change volcano jitter to points
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 to me.
Unfortunately 2 tests are failing.
Two tests (EditroConfig & Prettier) arent required any more and I adapted the settings accordingly.
mqc_stats <- read.table(file = paste0(wd,"/QC/multiqc_data/multiqc_samtools_stats.txt"), header=TRUE, sep="\t") | ||
columns <- c( | ||
"Sample", | ||
"raw_total_sequences", | ||
"reads_mapped_percent", | ||
"reads_duplicated_percent" | ||
) | ||
if (all(columns %in% colnames(mqc_stats))) { | ||
mqc_version <- 'new_mqc' | ||
} else { | ||
stop("Could not find a suitable multiqc table; please provide a correct multiqc.zip file or omit the parameter --multiqc altogether.") | ||
} |
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.
That looks like a copy of the code in lines 887-898. Generally it seems not good to have the same code twice, maintenance and so. If possible, might be better to solve that differently.
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 know, this is indeed not a nice code section but unfortunately I have to do this check twice, once inside a nested ifelse and once in a separate ifelse...
Co-authored-by: Daniel Straub <[email protected]>
Fix branch protection bug
second attempt at fixing branch protection -->merging directly as this changes no pipeline code and I need to see if the fix works
third attempt at fixing branch protection
Revert branch.yml changes
Fix path issue with report file
As https://nfcore.slack.com/archives/CE6SDBX2A/p1663077291333929 and https://nfcore.slack.com/archives/C02AL61NXPH/p1651131006534289 describe the same issue with the failing branch protection, and I could not find any solution there (and my attempts to fix it also failed), I'm going to ignore this error and merge. |
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).