search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
725 stars 38 forks source link

[REVIEW]: Minerva: a light-weight, narrative image browser for multiplexed tissue images #2579

Closed whedon closed 4 years ago

whedon commented 4 years ago

Submitting author: @thejohnhoffer (John Hoffer) Repository: https://github.com/labsyspharm/minerva-story Version: 1.0.0 Editor: @jni Reviewer: @will-moore, @sofroniewn Archive: 10.5281/zenodo.4088741

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

status

Status badge code:

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

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) by leaving comments 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

@will-moore & @sofroniewn, 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.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @jni know.

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Review checklist for @will-moore

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @sofroniewn

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @will-moore, @sofroniewn it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

:star: Important :star:

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

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf
whedon commented 4 years ago 
Reference check summary:

OK DOIs

- 10.1016/j.cels.2016.03.008 is OK
- 10.1016/j.cell.2020.03.053 is OK
- 10.1051/0004-6361/201322068 is OK
- 10.1158/2159-8290.CD-12-0095 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

jni commented 4 years ago

btw @sofroniewn @will-moore feel free to ping me with any questions about the process!

will-moore commented 4 years ago

I'm failing at "Is the source code for this software available at the repository url?" for 2 reasons:

jni commented 4 years ago

I think this means @thejohnhoffer needs to update the code in the repo to be readable, and include the code for minerva-author somewhere in the submission repo as well, but I hope @lpantano can confirm this or clarify...

jni commented 4 years ago

Just got confirmation from @arfon:

jni commented 4 years ago

@thejohnhoffer Did you see the comments above? I think at this point the reviewers are waiting on you to clarify where the code under review is.

thejohnhoffer commented 4 years ago

Hi @jni and @will-moore .

I'm sorry for the confusion. We have indeed split the code for this project among four separate repositories. Instructions for where to access these repositories is hidden on the Minerva Story Wiki, but I'll replicate that here.

Minerva Story

The GitHub Pages site build is stored at minerva-story. The source code for the minified bundle is stored at minerva-browser.

Minerva Author

The Python Flask server along with automated testing is stored at minerva-author. The React UI is stored at minerva-author-ui

Again, I do apologize. Minerva Story is the repository to host the final product in the form for a Jekyll blog, but Minerva as a whole contains much more informative code in the other three repositories.

will-moore commented 4 years ago

Hi @thejohnhoffer, thanks for that. Is there any more info on how to build the source code from https://github.com/labsyspharm/minerva-browser into the bundle.js at https://github.com/labsyspharm/minerva-story/blob/master/_includes/bundle.js? I can see a webpack build config etc for minerva-author-ui but not for minerva-browser? Many thanks, Will.

thejohnhoffer commented 4 years ago

Hi @will-moore ,

We have a webpack build config, but it has not yet been integrated into either minerva-story or minerva-browser. Today, I will refactor the code slightly to include in the minerva-story repo a proper build setup for the minerva-browser bundle.

thejohnhoffer commented 4 years ago

Hi @will-moore,

I've updated Minerva Story to include webpack instructions for rebuilding Minerva Browser.

will-moore commented 4 years ago

I've checked off all the items that I feel able to just now. I have issues setting up ruby on my Mac to test the install instructions, but I'll continue to look at that. Other issues are:

sofroniewn commented 4 years ago

I will start my review asap @jni, thanks @will-moore for ironing out some of those early issues!

jni commented 4 years ago

Just a status report from my perspective:

jmuhlich commented 4 years ago

Regarding the comparison to other tools, we thought the reference to our other publication Rashid 2020 (in review at another journal) would be sufficient -- that work surveys the field much more comprehensively than we have room for here. Would you prefer we include a small summary of that review in this manuscript? If not, we can at least make it more clear in the text that this information can be found in that reference.

We'll certainly add community guidelines and think about what we can create automated tests for.

will-moore commented 4 years ago

Hi @jmuhlich, Thanks, I see that Rashid et.al exactly ticks the box of "describe how this software compares to other commonly-used packages" but I guess that could be made a little clearer. I found that the DOI for Rashid seems to be incorrect: https://doi.org/10.1051/0004-6361/201322068 Google led me to https://www.biorxiv.org/content/10.1101/2020.03.27.001834v1 but I ended up reading the current version at https://www.biorxiv.org/content/10.1101/2020.03.27.001834v2. Thanks,

thejohnhoffer commented 4 years ago

I've updated the paper to mention that Rashid 2020 contains a comparison of existing software, I've also updated the DOI for that particular paper. We've also added contribution guidelines to the wiki. Also, I'd like to point out that we have tests for Minerva Author here and here that can be run with a pytest command.

@will-moore are you still having trouble install Ruby on MacOS? On our wiki, we have two options for installing Ruby on MacOS. The first option uses RVM, which can avoid a lot of the issues on MacOS with the system version of Ruby. If you're still stuck, would you be able to share the error you're seeing?

will-moore commented 4 years ago

Thanks @thejohnhoffer. The install for minerva-story and minerva-author worked fine for me. The only step missing from https://github.com/labsyspharm/minerva-author#installing that I had to do was brew install openslide. I was able to run the example from minerva-story. For minerva-author I can start the app but currently I never get further than Opening file: /path/to/image.tiff. This seems to hang for me. But don't think that needs to block any of my checklist, so I think I'm done.

jni commented 4 years ago

@whedon re-invite @sofroniewn as reviewer

whedon commented 4 years ago

OK, the reviewer has been re-invited.

@sofroniewn please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations

sofroniewn commented 4 years ago

Thanks @jni, permissions updated, I've begun the review process, including making some minor suggestions here https://github.com/labsyspharm/minerva-story/issues/63. Will press on with further review promptly

jni commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

sofroniewn commented 4 years ago

I've gone through the software paper and am happy with it, I think the added information that Rashid 2020 contains a comparison of existing software is sufficient to detail the state of the field.

I will continue to review the documentation and the functionality.

sofroniewn commented 4 years ago

I made an issue asking for the documentation links to be added to the readme so they are easier to discover.

I've gone through the documentation and functionality and found them to be satisfactory.

I found contributing guidelines here.

I wasn't able to find a full API reference say for code in https://github.com/labsyspharm/minerva-browser, but I think given the nature of this project as a web app, I don't think the target audience or potential contributors are expecting a full API reference and the code does contain helpful comments and appears clearly written, so I think it is sufficiently documented.

I wasn't able to find any continuous integration or automated tests, so I have left that checkbox blank, but I think that is not unusual for web projects of this nature and so I don't have any additional concerns.

I really like the examples present in the featured stories on the main software website. They are a great way to get a sense for what can be built with the tool and a highlight of the submission.

I have not made my own story, but I was able to install and successfully install and launch the software and am convinced by the high quality examples (which I have run in my browser) that the functionality exists as described in the manuscript and the performance of the software (interactive panning/ zooming of large datasets) matches the expectations described in the manuscript.

I now consider my review complete and approve this submission, but I'm happy to expand on any of my statements or look into things more at the request of @jni.

Great job authors, congrats on a really nice piece of software and manuscript. I'm excited to see the community pick this up!!

jni commented 4 years ago

Thank you @will-moore and @sofroniewn for your reviews!

@will-moore can you confirm that you're happy to accept the manuscript as it stands?

Thanks everyone for the interaction to improve the paper and the software thus far!

thejohnhoffer commented 4 years ago

I'd like to mention that to resolve issues with ruby dependencies, we have updated the manuscript and the Minerva Story repository to no longer require the use of Jekyll (or Ruby).

jmuhlich commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

will-moore commented 4 years ago

Hi, yes, I'm happy to accept the manuscript as it stands. Apologies for the delay in replying - been away with limited connection this week.

jni commented 4 years ago

@whedon accept

whedon commented 4 years ago

No archive DOI set. Exiting...

jni commented 4 years ago

LOL This is my first time accepting a JOSS paper so bear with me... 😅

jni commented 4 years ago

@whedon check references

whedon commented 4 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.cels.2016.03.008 is OK
- 10.1016/j.cell.2020.03.053 is OK
- 10.1101/2020.03.27.001834 is OK
- 10.1158/2159-8290.CD-12-0095 is OK

MISSING DOIs

- None

INVALID DOIs

- None
jni commented 4 years ago

@thejohnhoffer @jmuhlich I checked the final proof for typos/small errors, and found just two very minor issues:

After those small issues are fixed, we need you to make a tagged release and archive of the software, and to report the version number and DOI here. (That was Whedon's issue above 😂.)

thejohnhoffer commented 4 years ago

@jni I've fixed those two issues with the paper. Thanks for finding them.

For the archive process, our current idea is to use Zenodo to archive a zip file containing exports of our four separate repositories. We would version releases for our repositories separately, and we'd have a README in the Zenodo archive pointing to the four separate releases on github. The paper itself would be within the minerva-story folder of the Zenodo archive. Would this be an okay way to handle an archive for our four repositories?

jni commented 4 years ago

@thejohnhoffer great questions! To me it sounds good but I hope @lpantano and/or @arfon can provide a more definitive answer.

lpantano commented 4 years ago

I think that sounds good, I will let @arfon to re-confirm. As far as there is an isolated download from Zenodo that makes this reproducible to this point, it should be good (i think).

arfon commented 4 years ago

For the archive process, our current idea is to use Zenodo to archive a zip file containing exports of our four separate repositories. We would version releases for our repositories separately, and we'd have a README in the Zenodo archive pointing to the four separate releases on github. The paper itself would be within the minerva-story folder of the Zenodo archive. Would this be an okay way to handle an archive for our four repositories?

Sounds great. Thanks for confirming @thejohnhoffer.

thejohnhoffer commented 4 years ago

@jni @lpantano @arfon,

Thank you all. I've archived releases for all four repositories in one archive on on Zenodo.

jni commented 4 years ago

@whedon set 10.5281/zenodo.4088741 as archive

whedon commented 4 years ago

OK. 10.5281/zenodo.4088741 is the archive.

jni commented 4 years ago

@whedon set 1.0.0 as version

whedon commented 4 years ago

OK. 1.0.0 is the version.

jni commented 4 years ago

@whedon accept

whedon commented 4 years ago 
Attempting dry run of processing paper acceptance...
whedon commented 4 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.cels.2016.03.008 is OK
- 10.1016/j.cell.2020.03.053 is OK
- 10.1101/2020.03.27.001834 is OK
- 10.1158/2159-8290.CD-12-0095 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 4 years ago

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/1810

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/1810, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true