Joss-reviews: [REVIEW]: proEQUIB - IDL Library for Plasma Diagnostics and Abundance Analysis

Created on 17 Aug 2018  Β·  48Comments  Β·  Source: openjournals/joss-reviews

Submitting author: @danehkar (Ashkbiz Danehkar)
Repository: https://github.com/equib/proEQUIB
Version: v0.0.6
Editor: @arfon
Reviewer: @mdpiper, @mgalloy
Archive: 10.5281/zenodo.1890337

Status

status

Status badge code:

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

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

@mdpiper & @mgalloy, 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 @mdpiper

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: Does the release version given match the GitHub release (v0.0.6)?
  • [x] Authorship: Has the submitting author (@danehkar) 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 @mgalloy

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: Does the release version given match the GitHub release (v0.0.6)?
  • [x] Authorship: Has the submitting author (@danehkar) 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

@danehkar Thanks for addressing all the issues I raised. I've checked off the last item on my review checklist, so my review is complete!

All 48 comments

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

I created https://github.com/equib/proEQUIB/pull/1 to add a short note on installing the proEQUIB library. This addresses item 2 in the Documentation checklist.

Update: this issue has been resolved by @danehkar.

I added three issues:

to the author's repo to address the last three items in the Documentation checklist.

The equib_* routines in the pro directory don't appear to be written by the author. To separate them from the author's work, I suggest that they be moved to a new directory under externals. I've raised this as https://github.com/equib/proEQUIB/issues/5.

My review of proEQUIB is complete. The library and its associated paper are appropriate for publication in JOSS, pending @danehkar's responses to the issues I've opened in the repository. Further:

  1. https://github.com/equib/proEQUIB/issues/3 is especially important. There's a fair amount of code in the library (including a common block), and no tests. A set of tests that cover the library will increase my confidence that the library is working as it should.
  2. The paper reads well, but it could use one more editing pass. There are a couple awkward phrasings that could be smoothed; for example, "...allow to..." in the first and second bullets.

@arfon I'll complete the open items in the review checklist once the issues above have been addressed.

@arfon I'll complete the open items in the review checklist once the issues above have been addressed.

Excellent news, thanks @mdpiper! @danehkar - please let us know here when you've had a chance to incorporate this feedback.

I have checked off as many items on the Review Checklist as I can right now. Remaining items need to be addressed by @danehkar before I can check them off.

Friendly reminder to get to this review sometime soon please @mgalloy

As mentioned above, I have reviewed everything on the Review Checklist I can right now. The issues opened by @mdpiper need to be addressed by @danehkar to continue.

Friendly reminder to get to this review sometime soon please @mgalloy

Sorry, tagged the wrong person. Please let us know when you've had a chance to incorporate the feedback here @danehkar

@danehkar has confirmed by email that he is going to try and make updates by the end of November.

@mdpiper The equib_* routines in the pro directory don't appear to be written by the author. To separate them from the author's work, I suggest that they be moved to a new directory under externals. I've raised this as equib/proEQUIB#5.

I moved them to to a misc under externals, and I renamed equib_* to _*. These routines (interp2d.pro, meshgrid.pro, sign.pro, str2int.pro, strnumber.pro) were chosen from other IDL packages to replace some FORTRAN functions of EQUIB during my conversion from FORTRAN to IDL. I added equib_* to them to separate from other IDL functions with similar names in other IDL packages in my machine. Now I closed the issue equib/proEQUIB#5.

@mdpiper I added three issues:

to the author's repo to address the last three items in the Documentation checklist.

@danehkar Looks good! You can close https://github.com/equib/proEQUIB/issues/4. It would be best to host the documentation at another location, such as rtfd.io or your own site.

@mdpiper It would be best to host the documentation at another location, such as rtfd.io or your own site.

The documentation is now hosted at https://equib.github.io/proEQUIB/doc/

Please remove me from this and other JOSS reviewing threads, I have tried
but not managed to do that myself

Bo Sundman

Den lΓΆr 1 dec. 2018 kl 08:46 skrev Ashkbiz Danehkar <
[email protected]>:

@mdpiper https://github.com/mdpiper It would be best to host the
documentation at another location, such as rtfd.io or your own site.

The documentation is now hosted at https://equib.github.io/proEQUIB/doc/

β€”
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/openjournals/joss-reviews/issues/899#issuecomment-443407579,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AH3MdsNrYzKSNwllBE_h9c5hvbjf4tk-ks5u0jPOgaJpZM4WBW6i
.

I noticed that triangulate and trigrid in GDL have some bugs, and not work correctly in interp2d.pro. I replaced interp2d.pro with interp_2d.pro. Now proEQUIB functions also work in GDL as seen in Travis CI and Appveyor. To check, you need to install GDL (sudo dnf install gdl in Fedora, sudo apt-get install gnudatalanguage in Ubuntu, brew install gnudatalanguage in OS X).

@sundmanbo Unwatch this repo. Go to any page on for the JOSS reviews repo (such as the page for this thread), go to the top of the page, click "Unwatch".

@sundmanbo - yes, please do this:

I have marked all items on the checklist. My review is complete, good luck @danehkar!

I added three issues:

to the author's repo to address the last three items in the Documentation checklist.

@mdpiper I included mgunit in the folder test_mgunit for automated tests. You need to have atomic data in externals/atomneb/atomic-data and externals/atomneb/atomic-data-rc in do this test. To have all dependencies, you can clone the whole project with dependent libraries:

git clone --recursive https://github.com/equib/proEQUIB.git

Then, you can run the test for mgunit:

idl test_all.pro

There should not be any failure in test-results.log and test-results.html files.

I closed the issues equib/proEQUIB#2 and equib/proEQUIB#4. If there is no problem in the above test, I will close the issue equib/proEQUIB#3.

I have marked all items on the checklist. My review is complete, good luck @danehkar!

@mgalloy Thank you for your test. I much appreciate your review. I am now checking the paper if there is any typo there.

@arfon I uploaded the updated paper into the folder paper. Previously, there were some issues in paper.bib and the paper pdf file did not show the journal names.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

How is this looking @danehkar ☝️

@arfon The paper looks fine, but there is an issue in the citation order and style:

  • For [see e.g. @Danehkar:2013; @Danehkar:2014; @Danehkar:2016; @Danehkar:2018]:

    (Danehkar, 2018a; see e.g. Danehkar, Parker, & Ercolano, 2013; Danehkar, Parker, & Steffen, 2016; Danehkar, Todt, Ercolano, & Kniazev, 2014)

    should be:

    (see e.g. Danehkar, Parker, and Ercolano 2013; Danehkar et al. 2014; Danehkar, Parker, and Steffen 2016; Danehkar 2018a)

  • For [@Ercolano:2003; @Ercolano:2005]:

    (Ercolano, Barlow, & Storey, 2005; Ercolano, Barlow, Storey, & Liu, 2003)

    should be

    (Ercolano et al. 2003; Ercolano, Barlow, and Storey 2005)

  • For [@Danehkar:2016; @Danehkar:2018]:

    (Danehkar, 2018a; Danehkar et al., 2016)

    should be

    (Danehkar, Parker, and Steffen 2016; Danehkar 2018a)

  • For [see e.g. @Danehkar:2013; @Danehkar:2014; @Danehkar:2014b]:

    (Danehkar, 2014; see e.g. Danehkar et al., 2013, 2014)

    should be

    (see e.g. Danehkar, Parker, and Ercolano 2013; Danehkar et al. 2014; Danehkar 2014)

  • I do not think it is necessary to show more than 3 authors in text:

    (Arabas, Schellens, Coulais, Gales, & Messmer, 2010; Coulais et al., 2010)

    should be

    (Arabas, et al., 2010; Coulais et al., 2010)

The citation style in the previous PDF file did not have any problems.

Hi @danehkar, we've recently switched to the APA citation style which is likely the cause of many of these changes.

I'm afraid I don't have the bandwidth to work through all of these with you but for example, this set of citations:

[see e.g. @Danehkar:2013; @Danehkar:2014; @Danehkar:2016; @Danehkar:2018]

can be easily changed to:

(see e.g. @Danehkar:2013; @Danehkar:2014; @Danehkar:2016; @Danehkar:2018)

You can debug the output over in
https://github.com/openjournals/joss-reviews/issues/798 by making changes to your paper and then asking Whedon to recompile with @whedon generate pdf

@arfon I now switched to the APA citation style. I checked it here #798. You can reproduce the article proof. Thanks.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@danehkar Thanks for addressing all the issues I raised. I've checked off the last item on my review checklist, so my review is complete!

Great, thanks @mdpiper!

@danehkar - 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.

@mdpiper @mgalloy Thank you for reviewing this software.

@arfon Thank you for handling and editing this submission. I made an archived of the reviewed software in Zenodo: doi:10.5281/zenodo.1890337

@whedon set 10.5281/zenodo.1890337 as archive

OK. 10.5281/zenodo.1890337 is the archive.

@whedon accept

Attempting dry run of processing paper acceptance...

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

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/98, 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/99
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.00899
  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...

@mdpiper, @mgalloy - many thanks for your reviews here ✨

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

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

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

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