Joss-reviews: [REVIEW]: stingray: A modern Python library for spectral timing

Created on 18 Apr 2019  Â·  43Comments  Â·  Source: openjournals/joss-reviews

Submitting author: @dhuppenkothen (Daniela Huppenkothen)
Repository: https://github.com/StingraySoftware/stingray
Version: v0.1.3
Editor: @arfon
Reviewer: @Cadair
Archive: 10.5281/zenodo.3242835

Status

status

Status badge code:

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

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

@Cadair, 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 @arfon know.

✨ Please try and complete your review in the next two weeks ✨

Review checklist for @Cadair

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.3
  • [x] Authorship: Has the submitting author (@dhuppenkothen) 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)?
AAS accepted published recommend-accept review

All 43 comments

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @Cadair 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...

@Cadair - please carry out your review in this issue by updating the checklist above and giving feedback in this issue. The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html

Any questions/concerns please let me know.

:wave: @Cadair - how are you getting along here? Do you think you might be able to complete your review in the next week?

Hey, sorry for the delay I should be able to get to this hopefully by the end of the week :smile:

@dhuppenkothen Really glad to stingray hit a major milestone :smile:

A couple of notes from the review:

1) Should the citation for Astropy be the second paper from 2018, either instead of or in addition to the 2013 paper?
2) A few of your references in the paper seem to be missing links to DOIs.
3) I am not sure all your dependencies are installed automatically by pip so I opened StingraySoftware/stingray#397

I will have a play around with the code and complete my review as soon as I can (my train is about to arrive and I want to push send on this before it does :laughing:)

Oh also, do you plan to release the final 0.1 version before this paper is completed?

:wave: @Cadair - did you manage to complete your review yet?

@dhuppenkothen For some reason, I was sent the DOI of the AAS Journals submission of the Stingray paper instead of you.
So, here you go: 10.3847/1538-4357/ab258d

If you want, I can also send the entire email to you.

Thanks @1313e for posting it and letting us know it was sent to the wrong person -- I'll follow up with why that might have happened.

@dhuppenkothen - could you please add the following two fields to the paper.md YAML header:

aas-doi: 10.3847/1538-4357/ab258d
aas-journal: Astrophysical Journal

@crawfordsm @dhuppenkothen - is the associated AAS paper in ApJ?

This all looks great, apart from the queries I have above about dependencies, missing DOIs and the Astropy citation. :smile:

I had a response all typed out and then somehow forgot to actually send it. Thank you @Cadair for the comments!

To respond:

  1. We have fixed the citation (we now cite both, as per recommendation from one of the Astropy developers)
  2. We have added DOIs where we could. One is a paper in Astronomy and Astrophysics Supplements from 1993, which seems to not have a DOI I can find (the publisher points back to ADS for issues before 1996), and a couple of others are entries in ASCL, which also seem to not have DOIs associated with them.
  3. We're iterating with @Cadair on the dependencies, which I think is pretty close to merging.
  4. We have released Version 0.1 officially: https://zenodo.org/record/3239519#.XP6iky-ZMWo
  5. We have added the reference to the ApJ paper (and yes, the paper is indeed slated to be published in ApJ)

I realized I don't know whether the JOSS paper gets automatically updated from our master branch. Do I need to complete a separate step to make sure everything gets updated?

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon check references

Attempting to check references...

```Reference check summary:

OK DOIs

  • 10.1109/MCSE.2011.36 is OK
  • 10.1051/0004-6361/201322068 is OK
  • 10.3847/1538-3881/aabc4f is OK
  • 10.1088/0004-637X/770/2/103 is OK
  • 10.1117/12.2231304 is OK
  • 10.5281/zenodo.3239519 is OK

MISSING DOIs

  • None

INVALID DOIs

  • None
    ```

:+1: looks good to me.

We're iterating with @Cadair on the dependencies, which I think is pretty close to merging.

@dhuppenkothen - once this is merged, I think we're good to accept here.

Ok, @Cadair's PR has been merged. :)

Great! @dhuppenkothen - at this point could you:

  • Make a new release of Stingray that includes all of the changes from this review
  • Make an archive of the reviewed software in Zenodo/figshare/other service making sure that the Zenodo archive name matches the title of your paper and that the authors are the same as the paper?
  • Then update this thread with the DOI of the archive.

I can then move forward with accepting the submission.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

PDF failed to compile for issue #1393 with the following error:

/app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-18b9a37e7f54/lib/whedon/author.rb:23:in strip_footnotes': undefined method[]' for nil:NilClass (NoMethodError)
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-18b9a37e7f54/lib/whedon/author.rb:13:in initialize' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-18b9a37e7f54/lib/whedon.rb:141:innew'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-18b9a37e7f54/lib/whedon.rb:141:in block in parse_authors' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-18b9a37e7f54/lib/whedon.rb:138:ineach'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-18b9a37e7f54/lib/whedon.rb:138:in parse_authors' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-18b9a37e7f54/lib/whedon.rb:84:ininitialize'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-18b9a37e7f54/lib/whedon/processor.rb:36:in new' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-18b9a37e7f54/lib/whedon/processor.rb:36:inset_paper'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-18b9a37e7f54/bin/whedon:55:in prepare' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/command.rb:27:inrun'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in invoke_command' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor.rb:387:indispatch'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/base.rb:466:in start' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-18b9a37e7f54/bin/whedon:116:in from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in load' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in

'

Eek, I broke something. As I was preparing the Zenodo archive, I realized that I'd messed up the co-authors compared to the ApJ version. Trying to fix this, I apparently broke the paper? Any advice, @arfon?

@dhuppenkothen - this PR should fix it https://github.com/StingraySoftware/stingray/pull/412

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

Thank you, @arfon! I hope this is all correct this time around. I made a new release and synched it to Zenodo.

The Zenodo DOI is 10.5281/zenodo.3242835.

@whedon set v0.1.3 as version

OK. v0.1.3 is the version.

@whedon set 10.5281/zenodo.3242835 as archive

OK. 10.5281/zenodo.3242835 is the archive.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@Cadair many thanks for your review here ✨

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

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

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

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