Joss-reviews: [REVIEW]: straditize: Digitizing stratigraphic diagrams

Created on 28 Jan 2019  ยท  45Comments  ยท  Source: openjournals/joss-reviews

Submitting author: @Chilipp (Philipp S. Sommer)
Repository: https://github.com/Chilipp/straditize
Version: v0.1.1
Editor: @yochannah
Reviewer: @ixjlyons, @sgrieve
Archive: 10.5281/zenodo.2565036

Status

status

Status badge code:

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

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

@ixjlyons & @sgrieve, 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 @yochannah know.

โœจ Please try and complete your review in the next two weeks โœจ

Review checklist for @ixjlyons

Conflict of interest

Code of Conduct

General checks

  • [x] Repository: Is the source code for this software available at the repository url?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [x] Version: v0.1.1
  • [x] Authorship: Has the submitting author (@Chilipp) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] 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

  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] 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

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

Review checklist for @sgrieve

Conflict of interest

Code of Conduct

General checks

  • [x] Repository: Is the source code for this software available at the repository url?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [x] Version: v0.1.1
  • [x] Authorship: Has the submitting author (@Chilipp) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] 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

  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] 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

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

Most helpful comment

Dear @yochannah, @sgrieve and @ixjlyons,

thanks again for all the input and your fast responses! All reported issues (https://github.com/Chilipp/straditize/issues/1, https://github.com/Chilipp/straditize/issues/2, https://github.com/Chilipp/straditize/issues/3, https://github.com/Chilipp/straditize/issues/4 and https://github.com/Chilipp/straditize/issues/8) are closed and from my side, I am ready to make a new release with a fresh DOI. Is this okay for you?

All 45 comments

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @ixjlyons, it looks like you're currently assigned as the reviewer for this paper :tada:.

: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
Attempting PDF compilation. Reticulating splines etc...

๐Ÿ‘‰ Check article proof ๐Ÿ“„ ๐Ÿ‘ˆ

seems all right to me. Thanks at @sgrieve and @ixjlyons for being willing to review this package. Looking forward to your feedbacks!

Ok, my review is complete. Nice work @Chilipp!

Overall, straditize appears to be in excellent shape. Aside from a couple installation issues (filed against the target repository), all of the review criteria appear to me to be met. Beyond those, the code looks clean and well-written. I'm not all that familiar with digitization, but adding to the kinds of plots that can be "reverse-engineered" definitely seems worthwhile. The paper conveys that sentiment well and summarizes the package adequately. The documentation looks good standalone, but it shines inside the actual GUI and the integration with the tutorials is excellent.

Once the two installation issues I ran into are addressed, I recommend this submission be accepted.

Dear @ixjlyons, thank you very much for your time and your helpful feedback! I already implemented the suggested changes will merge them soon.

Hi @Chilipp, @yochannah Below is my review, as noted below I have also opened an issue in the straditize repo regarding the tests.

Firstly I would like to thank the author for this contribution. I have been very impressed by the quality of the software presented here, as well as it's potential utility to a wide range of users across disciplines. I am happy to recommend publication, pending the resolution of the issues outlined below.

Installation

I am performing this review on MacOS Sierra 10.12.6, and found no problems with the conda install as outlined in the documentation. However, one thing I did note is that the quick install instructions on the GitHub readme does not recommend installing in a separate virtual environment. I feel that installing into a virtual environment as the default quickstart install option is probably better than potentially causing problems in someones base environment.

I had the same problem as @ixjlyons when trying to install via pip and from source, that I needed to manually install PyQt. It seems that you have already addressed this, but I just wanted to highlight that it was an issue on my environment as well.

Functionality

Having worked through the tutorials I am happy to confirm that straditize works as described. More generally, I believe that this will be a significant tool for scientists in a range of disciplines, and the way that the software has been designed suggests to me that it will be straightforward to add more diagram types to straditize in future based on community need.

Documentation

I am very impressed by the level and quality of the documentation, in particular the integration of the documentation and tutorials into the GUI. Aside from the broken links mentioned elsewhere in this review, I noted a typo on the first line of this section: https://straditize.readthedocs.io/en/latest/installing.html#running-the-tests

We us should be We use.

Community guidelines

I just wanted to put a note here that your contribution/community guidelines are excellent. I will return to these the next time I have to write some for one of my projects. As above, I noted some broken links, but these have all been addressed in PR 7.

Tests

I have not been able to get the tests to pass on my machine. I have opened an issue to outline this further: https://github.com/Chilipp/straditize/issues/8

It may be useful to add some detail of what a user can expect when running these tests, for example the number of tests that will be run, what does a successful test run look like, are any tests skipped, the approximate duration of the tests.

Paper

The paper is very well written and clearly outlines the scientific problem the author is trying to address, as well as how straditize specifically addresses these issues. The format of the paper follows the general JOSS style, and I believe that all of the relevant literature and materials have been correctly cited.

In conclusion, I have thoroughly enjoyed the experience of reviewing this paper and would like to commend the author for their diligent work to address a pressing concern in our field. I am hoping to introduce this tool to some of my students in the future and look forward to using it in my own research. :rocket:

Wow, thanks @ixjlyons and @sgrieve for your thorough reviews, and @Chilipp for acting on the feedback so swiftly!

@sgrieve @Chilipp let me know when you two have managed to resolve the testing issue and are ready to go ahead! ๐Ÿ‘

Dear @yochannah, @sgrieve and @ixjlyons,

thank you very much for your nice words, helpful feedbacks and the reported issues. I apologize for the inconveniences with pip and also with the tests @sgrieve. Unfortunately I have no good methodology to test such a GUI-based application. The unittest framework requires creating and closing lot's of widgets which produces quite some overhead (this especially causes problems on the CI services with very limited RAM, such as OSX on Travis). I would like to address these problems first before merging the suggested changes. But this should be done by Monday.

Dear @yochannah, @sgrieve and @ixjlyons,

thanks again for all the input and your fast responses! All reported issues (https://github.com/Chilipp/straditize/issues/1, https://github.com/Chilipp/straditize/issues/2, https://github.com/Chilipp/straditize/issues/3, https://github.com/Chilipp/straditize/issues/4 and https://github.com/Chilipp/straditize/issues/8) are closed and from my side, I am ready to make a new release with a fresh DOI. Is this okay for you?

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon check references

Attempting to check references...

```Reference check summary:

OK DOIs

MISSING DOIs

  • None

INVALID DOIs

  • None
    ```

Okay, I've given it a finalish proofread and the text looks great ๐Ÿ’ฏ . One strange thing I noted is that the figure appears in the middle of the reference section though - @arfon any idea what is up?

@sgrieve and @ixjlyons, y'all each have one checkbox left unticked. If you're happy to accept, can you check those boxes and make a comment once you're done?

@Chilipp Once those last two checkboxes are complete, please do make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive! It's probably worth giving the manuscript a final proofread if you feel you need to, as well. ๐Ÿ‘

Thanks all, it's been really positive watching this all come together! ๐Ÿฅ‡

@arfon any idea what is up?

@yochannah - this is standard LaTeX image layout issues. If the authors make this change the image placement is better: https://github.com/Chilipp/straditize/pull/9

Thanks @arfon! The PR has been merged.

@whedon generate pdf

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@yochannah I checked the last box assuming @Chilipp will make a release on PyPI soon based on the recent fixes so pip install straditize Just Works™ per the installation instructions. Should a new release be made before getting a DOI?

@yochannah I've checked the final box, after the issue in the straditize repo was closed I forgot to do it...

All right @yochannah, I made a new version 0.1.1 that is available on PyPi and conda and created a release on zenodo:

DOI

It's probably worth giving the manuscript a final proofread if you feel you need to, as well

Also for the manuscript everything looks all right.

@whedon commands

Here are some things you can ask me to do:

# List all of Whedon's capabilities
@whedon commands

# Assign a GitHub user as the sole reviewer of this submission
@whedon assign @username as reviewer

# Add a GitHub user to the reviewers of this submission
@whedon add @username as reviewer

# Remove a GitHub user from the reviewers of this submission
@whedon remove @username as reviewer

# List of editor GitHub usernames
@whedon list editors

# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers

# Change editorial assignment
@whedon assign @username as editor

# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive

# Set the software version at the top of the issue e.g.
@whedon set v1.0.1 as version

# Open the review issue
@whedon start review

EDITORIAL TASKS

# Compile the paper
@whedon generate pdf

# Compile the paper from alternative branch
@whedon generate pdf from branch custom-branch-name

# Remind an author or reviewer to return to a review after a
# certain period of time (supported units days and weeks)
@whedon remind @reviewer in 2 weeks

# Ask Whedon to accept the paper and deposit with Crossref
@whedon accept

# Ask Whedon to check the references for missing DOIs
@whedon check references

@Chilipp okay, looking good - one last thing before we accept - can you make sure the Zenodo archive title is the same as the paper title? Thanks!

@whedon set v0.1.1 as version

OK. v0.1.1 is the version.

@Chilipp okay, looking good - one last thing before we accept - can you make sure the Zenodo archive title is the same as the paper title? Thanks!

All right, done

https://zenodo.org/record/2565036#.XGmbPNF7mM8

@whedon set 10.5281/zenodo.2565036 as archive

OK. 10.5281/zenodo.2565036 is the archive.

@openjournals/joss-eics - we're ready to accept this one!! ๐ŸŽ‰ ๐Ÿ†

@Chilipp Thanks for your great work on this package and fantastic collaboration with the reviewers.

@sgrieve @ixjlyons Thank you for your fantastic and in-depth reviews.

Thank you very much @yochannah and especially @sgrieve and @ixjlyons for your helpful inputs! I am very glad that everything worked out so well ๐Ÿ˜ƒ

@whedon accept

Attempting dry run of processing paper acceptance...

```Reference check summary:

OK DOIs

MISSING DOIs

  • None

INVALID DOIs

  • None
    ```

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

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

@whedon accept deposit=true

Doing it live! Attempting automated processing of paper acceptance...

๐Ÿšจ๐Ÿšจ๐Ÿšจ THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! ๐Ÿšจ๐Ÿšจ๐Ÿšจ

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/joss-papers/pull/502
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.01216
  3. If everything looks good, then close this review issue.
  4. Party like you just published a paper! ๐ŸŽ‰๐ŸŒˆ๐Ÿฆ„๐Ÿ’ƒ๐Ÿ‘ป๐Ÿค˜

    Any issues? notify your editorial technical team...

@ixjlyons, @sgrieve - many thanks for your reviews and to @yochannah for editing this submission โœจ

@Chilipp - your paper is now accepted into JOSS :zap::rocket::boom:

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

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.01216/status.svg)](https://doi.org/10.21105/joss.01216)

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

reStructuredText:
.. image:: http://joss.theoj.org/papers/10.21105/joss.01216/status.svg
   :target: https://doi.org/10.21105/joss.01216

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:

Was this page helpful?
0 / 5 - 0 ratings