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]: psrqpy: a python interface for querying the ATNF pulsar catalogue #538

Closed
18 tasks done
whedon opened this issue Jan 18, 2018 · 25 comments
Closed
18 tasks done
Assignees
Labels
accepted published Papers published in JOSS recommend-accept Papers recommended for acceptance in JOSS. review

Comments

@whedon
Copy link

whedon commented Jan 18, 2018

Submitting author: @mattpitkin (Matthew Pitkin)
Repository: https://github.com/mattpitkin/psrqpy
Version: v0.4.3
Editor: @arfon
Reviewer: @ygrange
Archive: 10.5281/zenodo.1175303

Status

status

Status badge code:

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

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

@ygrange, 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.

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 (v0.4.3)?
  • Authorship: Has the submitting author (@mattpitkin) 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
Copy link
Author

whedon commented Jan 18, 2018

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @ygrange 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
Copy link
Author

whedon commented Jan 18, 2018

Attempting PDF compilation. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Jan 18, 2018

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

@ygrange
Copy link

ygrange commented Jan 26, 2018

Can one of the editors elaborate on the meaning/background of the question: "Version: Does the release version given match the GitHub release (v0.4.3)?"
Seems like the author has released 0.4.4 by now byt 0.4.3 is still an existing release, even though the paper is part of the master repository and will therefore be part of any future release (which I don't think is a bad thing per se by the way).

@arfon
Copy link
Member

arfon commented Jan 26, 2018

@ygrange - we just want to make sure we're reviewing something close to the current version. I think it's fine to go ahead and look at the v0.4.4 release for your review. Also, yes, the version number often updates as a result of the review feedback (and the fact that a paper is now present in the repository).

@mattpitkin
Copy link

@ygrange Sorry, I had meant to say that I'd made a minor update and updated the release number. I added one function that allows you to download the whole ATNF catalogue and return it as an astropy table.

@ygrange
Copy link

ygrange commented Jan 26, 2018

Thanks for the clarification @mattpitkin !
Another question to the editor. Is it ok from an ethical perspective for me do do a pull request on the repository if I find an issue I can easily fix?

@ygrange
Copy link

ygrange commented Jan 30, 2018

Installation: seems to fail when using python2.7 because of astropy dependency. See ticket:
mattpitkin/psrqpy#12
Issue seems small to fix. Will continue with the quick fix I came up with.

@ygrange
Copy link

ygrange commented Jan 31, 2018

The paper is short and the tool is not very complex. I think this is typically one of those relatively small tools that can turn out to be very useful for people in the community. I have a few comments:

  • I couldn't chech the "community guidelines" box yet. Could you read what it states and add such a statement? I guess a single sentence in the README claiming that people wanting to contribute can do a pull request or open a ticket. Also I would add a sentence claiming that all code contributed in this way will adhere to the open source license of the project.
  • The documentation does not explicitly state what the exact target audience is and what problem it tries to solve. To me as a reviewer it is rather obvious from the package description what those two are (target audience: astronomers who want to use data from the ATNF pulsar catalogue in scripts; problem to solve: a web interface to the catalog is not exactly the best way to interact with the catalog from scripts; you want a better interface, with a stable API). It may be sensible for non-astronomer readers of the journal to add a sentence of two on this in both the article and the documentation.
  • You may want to put an example where the input and output are printed in the README. Alternatively it would be nice to have an integration test that essentially queries a well-known object with one or a few properties that aren't changing anytime son and compare the output to a reference (this could then be added to the tests you do in Travis).
  • I can't find an example of the usage of adsref. Could you add one?
  • The readthedocs is complete and the API is quite well-described. What I miss here is a description of the classes (Querying, Pulsars, and utility functions; the Setup constants seems obvious to me). Could you add a few sentences describing in short what each the class is meant to do so?

@mattpitkin
Copy link

@ygrange - thanks very much for the useful feedback. I'll try and address your points as soon as possible.

@mattpitkin
Copy link

I've added a statement about development and support to both the README and documentation.

mattpitkin added a commit to mattpitkin/psrqpy that referenced this issue Feb 7, 2018
@mattpitkin
Copy link

I've added a statement to the article and documentation about the intended users and purpose of the script.

mattpitkin added a commit to mattpitkin/psrqpy that referenced this issue Feb 7, 2018
…the travis build that the Crab pulsar's frequency rounds down to 29 Hz (this should be good for approximately 80 years)

 - refs openjournals/joss-reviews#538
@mattpitkin
Copy link

I've added some actual output to the README file and added a test in the travis build that queries the frequency of the Crab pulsars and checks that it rounds down to 29 Hz (this should be good for another 80 years).

mattpitkin added a commit to mattpitkin/psrqpy that referenced this issue Feb 7, 2018
@mattpitkin
Copy link

I've added an example to the documentation on getting an ADS reference link for a pulsar parameter. I've also added a bit more (although to be honest not much more) documentation for the QueryATNF and Pulsar classes. After adding the additional docs I bumped the version number up to 0.4.6.

@mattpitkin
Copy link

@ygrange - I hope my above changes address you main issues. Let me know if there's anything else that you'd like to see added.

@ygrange
Copy link

ygrange commented Feb 16, 2018

Very minor comment: In the usage description the word "txet" appears.

I think for the rest, the paper is good to go as far as I am concerned.

mattpitkin added a commit to mattpitkin/psrqpy that referenced this issue Feb 16, 2018
@mattpitkin
Copy link

Very minor comment: In the usage description the word "txet" appears.

Thanks, I've fixed to typo now.

@arfon
Copy link
Member

arfon commented Feb 18, 2018

@ygrange - just confirming we're good to accept here now?

@ygrange
Copy link

ygrange commented Feb 18, 2018 via email

@arfon arfon added the accepted label Feb 18, 2018
@arfon
Copy link
Member

arfon commented Feb 18, 2018

@mattpitkin - At this point could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? I can then move forward with accepting the submission.

@mattpitkin
Copy link

@arfon - sure, I've created an archive on Zenodo, with the DOI 10.5281/zenodo.1175303. DOI

@arfon
Copy link
Member

arfon commented Feb 19, 2018

@whedon set 10.5281/zenodo.1175303 as archive

@whedon
Copy link
Author

whedon commented Feb 19, 2018

OK. 10.5281/zenodo.1175303 is the archive.

@arfon
Copy link
Member

arfon commented Feb 19, 2018

@ygrange - many thanks for your review here ✨

@mattpitkin - your paper is now accepted in JOSS and your DOI is https://doi.org/10.21105/joss.00538 ⚡️ 🚀 💥

@arfon arfon closed this as completed Feb 19, 2018
@whedon
Copy link
Author

whedon commented Feb 19, 2018

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

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

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

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:

@whedon whedon added published Papers published in JOSS recommend-accept Papers recommended for acceptance in JOSS. labels Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted published Papers published in JOSS recommend-accept Papers recommended for acceptance in JOSS. review
Projects
None yet
Development

No branches or pull requests

4 participants