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

[REVIEW]: RNAsik: A Pipeline for complete and reproducible RNA-seq analysis that runs anywhere with speed and ease #583

Closed
whedon opened this issue Feb 14, 2018 · 69 comments
Assignees
Labels

Comments

@whedon
Copy link
Collaborator

@whedon whedon commented Feb 14, 2018

Submitting author: @serine (Kirill Tsyganov)
Repository: https://github.com/MonashBioinformaticsPlatform/RNAsik-pipe
Version: 1.4.9
Editor: @pjotrp
Reviewer: @andrewyatz
Archive: 10.5281/zenodo.1403976

Status

status

Status badge code:

HTML: <a href="http://joss.theoj.org/papers/32d910cf7f5acf29c9b49b82dfb647ce"><img src="http://joss.theoj.org/papers/32d910cf7f5acf29c9b49b82dfb647ce/status.svg"></a>
Markdown: [![status](http://joss.theoj.org/papers/32d910cf7f5acf29c9b49b82dfb647ce/status.svg)](http://joss.theoj.org/papers/32d910cf7f5acf29c9b49b82dfb647ce)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@andrewyatz, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @pjotrp know.

Conflict of interest

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Version: Does the release version given match the GitHub release (1.4.9)?
  • Authorship: Has the submitting author (@serine) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

  • Authors: Does the paper.md file include a list of authors with their affiliations?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Feb 14, 2018

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @andrewyatz it looks like you're currently assigned as the reviewer for this paper 🎉.

⭐️ Important ⭐️

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Feb 14, 2018

Attempting PDF compilation. Reticulating splines etc...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Feb 14, 2018

@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented Feb 14, 2018

Hi @serine, thanks for your interesting submission. @andrewyatz will review. To expedite the process, can you go through above checklist and make sure we ought to be able to tick the boxes? And ping us here when you are done. Thanks!

@andrewyatz

This comment has been minimized.

Copy link

@andrewyatz andrewyatz commented Feb 16, 2018

Hi @serine I've been running your installation procedure and have two issues with it. Could you double check your installation scripts on a clean system and just make sure everything is in order please? I'd rather not be opening lots of issues on your repos if I can avoid it plus it lengthens the review process incredibly.

@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented Feb 19, 2018

Ping @serine. The reviewer needs to be able to run the software. Can you provide the documentation to do just that? Should be straightforward with JVM. If you have already done that and can tick above boxes (e.g. for contribution guidelines) spell it out for the readers. Thanks!

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Feb 21, 2018

@pjotrp and @andrewyatz as I've mention in my previous comment RNAsik depends on a lot of different tools and I some what assumed that dependencies of every tool don't need to be listed under RNAsik. In the case of BigDataScripit it depends on java, golang and ant, given that I said BigDataScript is required I assumed it dependencies.

I since updated the docs to reflect all major/crucial dependencies in the docs, that of course requires a few system installs. I've tested the docs out and it worked for me. If you follow quick start RNAsik should install. As I also mentioned in the docs now, I've only really tested this on ubuntu 16.04 (also works on 14.04). I don't have a mac so can't really test on that, but you wouldn't want to run it on mac anyway given the resources, unless the data is small of course e.g bacteria.

so like I said docs have been updated with new install instruction.

Let me know how it goes. also if you gonna meet some issues on mac do let me know so that I can accommodate mac users too.

thanks

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Feb 21, 2018

Just to follow up on the previous comment above. virtualenv isn't strictly necessary if you already have ansible installed, you can skip virtualenv step.

Every time you are opening a new shell you'll need to export both bds and RNAsik again. You can put those two export lines into your ~/.bashrc if you like. I personally don't like to pollute my ~/.bashrc with random exports.

A general comment on bio-ansible, which I think I already made somewhere, you can change bio.yml to all.ym and add --ask-become-pass which will prompt you for your sudo password. This will do bunch of apt-get installs (again, ubuntu specific) following by tools installation. The problem with doing all.yml this will install potentially many more apt-get packages than you really don't need for RNAsik

I realise now that this is rather involved installation process. So far this is the best we have. If you have any suggestion and/or comments don't hesitate to open an issue under RNAsik issues.

cheers

@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented Feb 21, 2018

Complicated tools may have complicated setups ;). One solution would be to provide a Docker container (though I am no fan particularly). @andrewyatz are up up for trying? Possibly a VM would be the easiest.

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Feb 22, 2018

yeh, I'm not sold on docker yet, but I can try getting you a docker image if that's preferred. Let me know @andrewyatz

@andrewyatz

This comment has been minimized.

Copy link

@andrewyatz andrewyatz commented Feb 22, 2018

I'm going to try normally. You have my upmost sympathy with respect to dependency management and bioinformatic programs.

@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented Feb 22, 2018

You may want to look into GNU Guix some day. I don't have any problems with the dependency graph.

@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented Mar 12, 2018

@andrewyatz how are you doing?

@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented Mar 16, 2018

Hi @serine. Looks like we have an impasse. Software deployment is the achilles heel of bioinformatics!

Can you look into a deployment scheme? To me Docker, bioconda or GNU Guix are the systems of choice. Apt should not be used because it interferes with other software on a system. It should be easy to run software - at least a version of it. One advantage of choosing one of the three that it includes dependencies and it that gets promoted for future pipelines too. There is a paper coming out soon on comparing work flows, including conda and Guix. If you are interested I can send you a pre-press.

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Mar 16, 2018

@pjotrp It is a shame that you guys having issues with running pipeline, but it is great to get this kind of feedback about "ease of use" so to speak. Like I said before I can't imagine any other pipeline will be any easier to install. At the end of the day the pipeline doesn't require any installation at all, just BigDataScript, that can be downloaded as pre-compiled binary. But of course the pipeline of no use if other tools aren't there.

I think docker beta ready. I'd have to dig into bio-ansible a bit (docker image is also build with bio-ansible, but you won't have to do it. I'll make it) I think internally we are would like to head bioconda way, in fact similar to docker, we are some what in alpha, but at the moment there is no conda channel for Golang and so there isn't a clean way of specifying dependency for BigDataScript at the moment.

I did have a look at Guix briefly, I wasn't convinced.. I get a little confused when package manager gets mixed in with distro.. similar to https://nixos.org/nix/ which is both a package manager and a distro..

Yes please I'd like the pre-print if this is ok. Need to be convinced that docker or bioconda is the way to go.. I personally like to compile from source hence bio-ansible, but do understand "ease of use" and quick access is more important

I have a few things piled up and am helping at running a python course next week. Also down here (Melbourne) we having Easter break from the 30th of March, so might be out of touch for a couple of week, but will try to get this all sorted and report back as soon as I can

Cheers

@andrewyatz

This comment has been minimized.

Copy link

@andrewyatz andrewyatz commented Mar 16, 2018

Hi. Sorry this is more my fault that it's just not been possible for me to devote enough time to test through the existing changes that @serine has implemented. Don't take this as a comment on the changes but my availability.

@andrewyatz

This comment has been minimized.

Copy link

@andrewyatz andrewyatz commented Mar 16, 2018

Okay I've resurrected my work and switched to an Ubuntu box. Still no joy. In fact the same error I see on my mac. This is in MonashBioinformaticsPlatform/RNAsik-pipe#22 with the error.

@andrewyatz

This comment has been minimized.

Copy link

@andrewyatz andrewyatz commented Apr 16, 2018

Sorry can I prod again on this. There's little to no chance of me installing this. As far as I am aware there's been no response to the issues I've raised. The error isn't limited to just my mac as otherwise my VM attempts would have worked. Not sure where else to push this now

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Apr 17, 2018

@andrewyatz sorry about that, I've gone quite, my fault. I just responded to your other issue could you have one more try. Thanks

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Apr 19, 2018

@andrewyatz It does appear that bio-ansible way to install RNAsik is a bit too much. Taking advice from @pjotrp and others around me I've created conda package, for linux only at this stage, which should simplify install great deal.

Could you have a look at updated docs

Basically you'll need to install miniconda and then RNAsik from my "channel" (conda terminology here)

bash Miniconda3-latest-Linux-x86_64.sh 
conda install -c serine rnasik 
conda install -c bioconda qualimap 

QualiMap is separate install, because it itself has fair few dependencies and it was tricky to include into RNAsik package

On my laptop it took about 35 minutes to install, this is subject to reasonably good internet connection as installation creates fair bit of traffic, downloading all of the different dependencies.

Hopefully this will simplify things for you and others

Cheers

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Apr 19, 2018

by the way this is my channel page https://anaconda.org/serine
and this is rnasik package page https://anaconda.org/serine/rnasik

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Apr 20, 2018

@andrewyatz sorry, I think you also need to added a couple of channel

conda config --add channels defaults
conda config --add channels conda-forge
conda config --add channels bioconda

before installing from conda

also updated docs.

p.s default location of the config is ~/.condarc

mine looks like this

[biostation2]~$ cat ~/.condarc 

channels:
  - bioconda
  - conda-forge
  - defaults
@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented May 25, 2018

@andrewyatz do you have time to look at this? We can look for another reviewer if necessary.

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented May 30, 2018

@pjotrp @andrewyatz Let me guys know if you need anything else from me.

Just to reiterate that the best way to install RNAsik is to use conda install. I actually shifted myself and now use conda install all the time, cause I tend to jump between diff machines.

Also just to let you know that RNAsik have moved since I submitted for review. I'm now up to 1.5.1 version and making 1.5.2 soonish.. there have been some changes, but all in backward compatible manner.

You can only get 1.5.1 through conda install, but that shouldn't effect the review right?

Cheers

@andrewyatz

This comment has been minimized.

Copy link

@andrewyatz andrewyatz commented May 30, 2018

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented May 31, 2018

@andrewyatz no problem, thanks for all the feedback that you've provided so far.

@pjotrp hopefully someone else can help out?

Cheers

@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented Jun 1, 2018

Alright, let's look for someone who can run this pipeline, so we can wrap up review.

@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented Jul 19, 2018

@arfon we are ready for acceptance. Can you shine your wisdom on above? Note to others: Arfon is still on holiday, so it may take a little while.

@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented Jul 19, 2018

And thanks @andrewyatz for your review! You did most of the hard work 👍

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Jul 19, 2018

great thanks a lot guys, and yes shout out to everyone involved, every little bit of advise, comment and gitissue really helped in shaping this into a better tool !

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Jul 19, 2018

@mr-c was that comment Would an RRID URI be acceptable? to me or at @arfon cause I'm happy with what ever works

@mr-c

This comment has been minimized.

Copy link

@mr-c mr-c commented Jul 19, 2018

@serine my comment about RRIDs was mostly to JOSS management. I'm just a random person with no special standing here :-)

But, while you're open to feedback from strangers, I would suggest splitting up the Summary section into multiple paragraphs. I find it hard to read as one big block of text.

https://github.com/openjournals/joss-papers/blob/joss.00583/joss.00583/10.21105.joss.00583.pdf

The template is also overlapping the page numbers with the footer.

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Jul 19, 2018

@mr-c how about now https://zenodo.org/record/1315013#.W1A5FHVuaV4 ?

using

pandoc -o check.pdf --bibliography=paper.bib paper.md

builds to this

check.pdf

not sure how it'll look with JOSS compilation.. also don't think I can do much about footer overlap

if you think this looks ok I can commit this change to master and perhaps ask @pjotrp to rebuild final pdf if that's ok?

@mr-c

This comment has been minimized.

Copy link

@mr-c mr-c commented Jul 19, 2018

@serine much more readable, thank you.

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Jul 19, 2018

thanks, here is commit dd567e44

@pjotrp would you be able to regenerate new JOSS pdf? thanks

@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented Jul 19, 2018

I think you can do it with

   @whedon generate pdf
@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Jul 19, 2018

@whedon generate pdf

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Jul 19, 2018

@whedon generate pdf pretty please ?

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Jul 19, 2018

Attempting PDF compilation. Reticulating splines etc...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Jul 19, 2018

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Jul 19, 2018

Attempting PDF compilation. Reticulating splines etc...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Jul 19, 2018

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Jul 19, 2018

haha always works if you ask nicely

@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented Aug 7, 2018

@arfon I think we are ready to R&R

@arfon

This comment has been minimized.

Copy link
Member

@arfon arfon commented Aug 7, 2018

@serine - you appear to have associated the JOSS DOI with your archive in Zenodo (https://zenodo.org/record/1315013#.W2m3Nv5Khjt). Could you please follow this guide and create an archive of the GitHub repository (https://github.com/MonashBioinformaticsPlatform/RNAsik-pipe) that has a Zenodo DOI and not a JOSS one?

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Aug 8, 2018

right, thanks for the link to the guide @arfon . I've done everything but the new release. I'll have to do a bit a work for my next release. I can't just slap new release tag there is a timeline and things :)
Will try to get it done in the next couple of days.

Pretty exciting to be so close !

Thanks

@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented Aug 23, 2018

Hi @serine, I think we ought to push this out.

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Aug 27, 2018

@pjotrp sorry about the wait, was away for a week and was playing catch up. I just made another release that is hooked to zenodo. https://zenodo.org/account/settings/github/repository/MonashBioinformaticsPlatform/RNAsik-pipe

I didn't realise before that I could have "annotated" any of my previous releases and that would have showed up in zenodo. I thought that after setting up a webhook zenodo was "waiting" for an event (next release). Anyway I think all done now.

@arfon is there anything else I need to do?

Thanks

@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented Aug 27, 2018

@arfon I believe we are good to accept this submission!

@pjotrp

This comment has been minimized.

Copy link

@pjotrp pjotrp commented Aug 27, 2018

@whedon set 10.5281/zenodo.1403976 as archive

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Aug 27, 2018

OK. 10.5281/zenodo.1403976 is the archive.

@serine

This comment has been minimized.

Copy link
Collaborator

@serine serine commented Aug 27, 2018

Awesome !!! thanks heaps everyone !

@arfon arfon added the accepted label Aug 27, 2018
@arfon

This comment has been minimized.

Copy link
Member

@arfon arfon commented Aug 27, 2018

@andrewyatz @mr-c - many thanks for your reviews here and to @pjotrp for editing this submission

@serine - your paper is now accepted into JOSS and your DOI is https://doi.org/10.21105/joss.00583 ⚡️ 🚀 💥

@arfon arfon closed this Aug 27, 2018
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Aug 27, 2018

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](http://joss.theoj.org/papers/10.21105/joss.00583/status.svg)](https://doi.org/10.21105/joss.00583)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.00583">
  <img src="http://joss.theoj.org/papers/10.21105/joss.00583/status.svg" alt="DOI badge" >
</a>

This is how it will look in your documentation:

DOI

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

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

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.