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

PR for Release 1.4 #300

Merged
merged 382 commits into from
Oct 15, 2019
Merged

PR for Release 1.4 #300

merged 382 commits into from
Oct 15, 2019

Conversation

apeltzer
Copy link
Member

@apeltzer apeltzer commented Oct 9, 2019

This is the dev to master PR for making a release 1.4 for nf-core/rnaseq. Lots of changes, so please have a look and review 👍

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 necessary, also make a PR on the nf-core/rnaseq branch on the nf-core/test-datasets repo
  • Ensure the test suite passes (nextflow run . -profile test,docker).
  • Make sure your code lints (nf-core lint .).
  • Documentation in docs is updated
  • CHANGELOG.md is updated
  • README.md is updated

drpatelh and others added 30 commits June 10, 2019 12:42
First pass update of relevant files for Salmon addition
Bug fixes for Salmon addition
Minor edit: the docker container name does not have a hyphen
Minor edit to installation.md
Minor: Update other singularity-related docs to reference the correct docker image.
Update adding_your_own.md
 add tximport and summarizeexperiment
Major overhaul of Salmon requirements
@apeltzer
Copy link
Member Author

index

@apeltzer
Copy link
Member Author

I guess the tools 1.7.1 release is something I will wait for (when we release very soon?) ...

apeltzer and others added 3 commits October 14, 2019 17:17
* Remove ToDo string from README

* Template update for nf-core/tools version 1.8.dev0

* Fix multiqc stuff

* Remove png, include fix by @drpatelh
Copy link
Contributor

@olgabot olgabot left a comment

Choose a reason for hiding this comment

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

Whooo so exciting! I had a few suggestions to make the documentation a little easier to understand. I'm also still confused about when the edgeR stuff is Euclidean distances vs Pearson's correlation.. maybe you can help?

apeltzer and others added 2 commits October 14, 2019 22:43
@apeltzer apeltzer requested a review from olgabot October 14, 2019 20:48
Copy link
Contributor

@olgabot olgabot left a comment

Choose a reason for hiding this comment

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

Yay it's all "Pearson's Correlation" now! phew I was really confused

@ewels
Copy link
Member

ewels commented Oct 15, 2019

Can’t remember why we didn’t just use Pearson’s in the first place... Sorry for any trauma that this decision caused over the years 😂

@olgabot
Copy link
Contributor

olgabot commented Oct 15, 2019

Eventually I think one could replace some k-mer based methods for sample similarity over the edgeR stuff and also make an MDS or UMAP plot but that is not in the scope of this release ...

apeltzer and others added 3 commits October 15, 2019 09:17
* Remove ToDo string from README

* Template update for nf-core/tools version 1.8.dev0

* Fix multiqc stuff

* Remove png, include fix by @drpatelh

* Add Memory adjustments
@apeltzer
Copy link
Member Author

Will get the container updated once more - preseq has been upgraded at bioconda now, rsem also compiling with R 3.6.1 now, so once this is done this is really good to go I believe 👍

Will then need another approval, e.g. @ewels or @drpatelh :-)

@drpatelh
Copy link
Member

Oh. Ok then. Ill have a look 👍

@apeltzer
Copy link
Member Author

See #314

Copy link
Member

@drpatelh drpatelh left a comment

Choose a reason for hiding this comment

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

Breezed over it @apeltzer given the reviews that have come before. Just a couple of minor comments/corrections 👍

I have to say we may need to think about splitting this pipeline up at some point because its gone way beyond the simplicity required to work out whats going on!

apeltzer and others added 3 commits October 15, 2019 13:17
* Remove ToDo string from README

* Template update for nf-core/tools version 1.8.dev0

* Fix multiqc stuff

* Remove png, include fix by @drpatelh

* Add Memory adjustments

* Final Dependency updates for PR 1.4

* Let's get RSEM in once we add it :-)
@apeltzer apeltzer dismissed ewels’s stale review October 15, 2019 13:08

Not up2date anymore - all things resolved 🎉

@apeltzer apeltzer merged commit 55eee0c into master Oct 15, 2019
--fasta '[path to Fasta reference]' \
--gtf '[path to GTF file]' \
--bed12 '[path to bed12 file]'
--star_index '/path/to/STAR/index' \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
--star_index '/path/to/STAR/index' \
--star_index '/path/to/star_index_directory/' \

Make it more obvious that this is a folder

--gtf '[path to GTF file]' \
--bed12 '[path to bed12 file]'
--star_index '/path/to/STAR/index' \
--hisat2_index '/path/to/HISAT2/index' \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
--hisat2_index '/path/to/HISAT2/index' \
--hisat2_index '/path/to/hisat2_index_directory/' \

Make it more obvious that this is a folder

Copy link
Contributor

Choose a reason for hiding this comment

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

oops I thought I submitted this!! next time ..

Copy link
Member Author

Choose a reason for hiding this comment

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

Something for V1.5 :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.