Joss-reviews: [REVIEW]: f90nml - A Python module for Fortran namelists

[whedon]

Submitting author: @marshallward (Marshall Ward)
Repository: https://github.com/marshallward/f90nml
Version: 1.1.2
Editor: @danielskatz
Reviewer: @zbeekman, @tclune
Archive: 10.5281/zenodo.3245482

Status

Status badge code:

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

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

@zbeekman & @tclune, 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 @danielskatz know.

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

Review checklist for @zbeekman

Conflict of interest

• [x] As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

• [x] I confirm that I read and will adhere to the JOSS 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: 1.1.2
• [x] Authorship: Has the submitting author (@marshallward) 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 @tclune

Conflict of interest

• [x] As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

• [x] I confirm that I read and will adhere to the JOSS 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: 1.1.2
• [x] Authorship: Has the submitting author (@marshallward) 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)?

[whedon]

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

1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

For a list of things I can do to help you, just type:

@whedon commands

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

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

[danielskatz]

👋 @zbeekman, @tclune - we're ready to go ahead with the review in this issue.

And I know @zbeekman won't do anything until May 31...

[danielskatz]

As stated in the first comment in this issue, please carry out your review in this issue by updating the your checklist (above).

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.)

Let me know if you have any problems or questions

[zbeekman]

I can't seem to tick any check boxes

[tclune]

Works for me. Of course, I first started checking @zbeekman 's boxes ... But I then unchecked them.

[danielskatz]

Quoting the comment above:

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

I'm sure the first is ok - can you make sure you did the second?

[zbeekman]

I'd accepted already for a previous review. It started working just now. Thanks!

[tclune]

I had one minor issue with the installation. 3 different methods are described, but the one that that avoids the requirement for root privileges did not work for me. I have amended a previously closed issue.

[tclune]

My only other negative comments are:

• I could not find guidelines for community contributions. OTOH, this is a vanilla GitHub project that should easily work with standard approaches of fork+PR or opening issues.
• The API documentation appears to be at best partially completed.

[marshallward]

Thanks very much for the feedback. On the three points:

• For installation, I'm not able to reproduce the problem you're seeing, including on my work machine where I do not have root access. But we can work through that in marshallward/f90nml#49. (Or perhaps I just ought to remove the explicit setup.py install?)

• I'll prepare a Contributing guidelines file. (Patches are so infrequent that I haven't had much need for one.)

• I'll review the API, but could you mention any examples of missing items? I should say that I only consider the Namelist and Parser classes, along with their public functions, as part of the public API. But if there's an expectation to fully document all functions and to integrate them into the documentation, then I'm happy to do that.

[tclune]

@marshallward Please disregard my API comment for now. I did not see the Makefile and was just staring at the raw files in the ./doc directory. I'm building the documentation and will update my comment.

[tclune]

I retract my comment about the API. Documentation is complete and nicely done. And I thank the author for getting me to actually look at sphinx. I'd heard of it, but never (knowingly) used it before.

[danielskatz]

👋 @zbeekman - How is your review coming along?

[zbeekman]

👋 @zbeekman - How is your review coming along?

Jumping in now.

[zbeekman]

@danielskatz:

• I'm waiting to check one box for https://github.com/marshallward/f90nml/issues/95
• I think https://github.com/marshallward/f90nml/issues/96 relates to installation and functionality. It should at least be documentented as a dependency, and/or a more graceful failure should occur when the yaml module is missing
• https://github.com/marshallward/f90nml/issues/97 is a pretty minor pedantic annoyance

Once community/contributing is addressed, I'm happy to check the last checkbox. IMO, the last two issues are not show-stoppers, but I would categorize them as minor changes requested.

I could comment on a number of the boxes that I checked during the review process to document my justification and thought process, if that would be useful. @danielskatz please let me know if you think I should do this and whether or not there are any additional tasks for me, beyond checking off the contributing/community box once upstream provides adequate documentation.

@marshallward Nice work!

[zbeekman]

Also, it looks like a lot of climate science tools/packages are using f90nml: https://github.com/marshallward/f90nml/network/dependents very cool!

[danielskatz]

@zbeekman:

I could comment on a number of the boxes that I checked during the review process to document my justification and thought process, if that would be useful.

No, you don't need to do this.

[zbeekman]

@danielskatz My review is finished.

I can confirm that the paper and the source code meet the review criteria determined by JOSS, and enthusiastically recommend this paper/package for publication. f90nml fills an important void for enabling mixed language Python/Fortran programming, more easily designing and executing simulation and analysis workflow pipelines, and easing legacy code modernization efforts.

My only remaining advice to the author is that the reliance of the package on the pyyaml module (marshallward/f90nml#96) could be better, or more prominently documented, and an error message pointing the user in the right direction if they try to produce yaml output without would be helpful. In addition, the link to the documentation in the README is well placed near the top of the document, however, creating a documentation header, even if only followed by a one-sentence paragraph, would make it more prominent.

Otherwise, the package is very well written, performs testing and installation in a very straightforward, pythonic, and canonical way. It includes an extensive automatic test suite with great code coverage, and thorough documentation.

If I can be of any additional assistance, please do not hesitate to ask.

Thanks!

-Izaak "Zaak" Beekman

[danielskatz]

Thanks @zbeekman!

[danielskatz]

👋 @marshallward - please take

My only remaining advice to the author is that the reliance of the package on the pyyaml module (marshallward/f90nml#96) could be better, or more prominently documented, and an error message pointing the user in the right direction if they try to produce yaml output without would be helpful. In addition, the link to the documentation in the README is well placed near the top of the document, however, creating a documentation header, even if only followed by a one-sentence paragraph, would make it more prominent.

as advice - if you want to make changes based on this, please do.

[marshallward]

Thanks @danielskatz, I am currently working through this and other issues with @zbeekman and @tclune in the GitHub issues cited above, it's been valuable feedback.

[danielskatz]

👋 @marshallward - I assume you are still working through issues? (just checking in...)

[marshallward]

Yes, only the YAML issue left at this point, which is done but not yet documented or uploaded to PyPI. Also waiting on some feedback on the README changes but that is not essential.

Was quite literally working on it when you posted, so should hopefully be done soon (excepting user feedback).

[marshallward]

Barring any exception from the reviewers, I think I'd say that the various issues raised have been resolved.

[danielskatz]

Thanks - @tclune and @zbeekman - back to you (mostly @tclune)

[zbeekman]

Yes, my review is complete, and I am completely satisfied. Thanks!

[tclune]

Yes - my review is complete and completely satisfied!

[danielskatz]

@tclune - can you check off the remaining items in your list in that case?

[danielskatz]

@marshallward - Can you make a deposit of the current repo into an archive (e.g. zenodo via https://guides.github.com/activities/citable-code/) and tell me the DOI? If you also need to update the version number, let me know that as well.

[tclune]

Done.

[marshallward]

@danielskatz the updated DOI via Zenodo is here: https://zenodo.org/record/3245482

10.5281/zenodo.3245482

I've done a version update to 1.1.2 which includes the reviewer-recommended changes, feel free to update it (I can't see a way to do it myself, but happy to do so if it's possible).

[danielskatz]

@whedon set 10.5281/zenodo.3245482 as archive

[whedon]

OK. 10.5281/zenodo.3245482 is the archive.

[danielskatz]

@whedon set 1.1.2 as version

[whedon]

OK. 1.1.2 is the version.

[danielskatz]

@whedon accept

[whedon]

Attempting dry run of processing paper acceptance...

[whedon]

```Reference check summary:

OK DOIs

• 10.1002/cpe.1535 is OK
• 10.1109/SE-HPCCSE.2014.7 is OK
• 10.5065/1dfh-6p97 is OK
• 10.1088/0953-8984/21/39/395502 is OK

MISSING DOIs

• None

INVALID DOIs

• None
  ```

[whedon]

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

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

[danielskatz]

@whedon accept deposit=true

[whedon]

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

[danielskatz]

thanks again @zbeekman and @tclune

[whedon]

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦

[whedon]

🚨🚨🚨 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/759
2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.01474
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...

[danielskatz]

I currently got a 404 back from https://www.theoj.org/joss-papers/joss.01474/10.21105.joss.01474.pdf
This usually works after a few minutes, but I'm just getting on a plane, so @arfon, if you see this working, please feel free to close this issue for the final publication step

[danielskatz]

It now works!

[whedon]

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

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

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

This is how it will look in your documentation:

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:

• Volunteering to review for us sometime in the future. You can add your name to the reviewer list here: http://joss.theoj.org/reviewer-signup.html
• Making a small donation to support our running costs here: https://numfocus.salsalabs.org/donate-to-joss

[marshallward]

I'm still getting a 404, but may be solved with patience.

Also, the recommended citation also seems to be missing my first initials, but perhaps this is standard?

Anyway, don't miss your flight over these details :)

[danielskatz]

@arfon, can you help with the initials issue?

[danielskatz]

Yes, the 404 is a caching issue. Once it fails, you need to wait a bit for it to resolve. For me, it failed on airport Wi-Fi but worked a few minutes later via phone network

[marshallward]

Must be something like that, I can see the file directly here:

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

As for the initials, I did notice that my codemeta.json file does not include my middle initial, which means it must have pulled the info from my GitHub account or git configs. I think I used one of the Ruby tools to generate that. I can fix it, though not sure how post-publish edits are handled here.

Though having said all that, the recommended citation also lacks my first initial, so still might be something that needs to be fixed.

[marshallward]

I've just looked at a few others and their "Cite as:" also lacks first initials, so perhaps this is something to be handled outside of this issue :).

[marshallward]

Also, for what it's worth, I found a typo in the paper, p2 L14 itepreted -> interpreted, and have edited my paper.md, but no worries if it's too late to resolve it.

[arfon]

I've just looked at a few others and their "Cite as:" also lacks first initials, so perhaps this is something to be handled outside of this issue :).

Yeah, this isn't unique to this paper, this is currently how we create the recommended citations for single-author papers.

I've opened a bug report here but I don't expect a quick fix sorry.

Also, for what it's worth, I found a typo in the paper, p2 L14 itepreted -> interpreted, and have edited my paper.md, but no worries if it's too late to resolve it.

I can fix this.

[arfon]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[arfon]

Also, for what it's worth, I found a typo in the paper, p2 L14 itepreted -> interpreted, and have edited my paper.md, but no worries if it's too late to resolve it.

OK, the PDF is updated in https://github.com/openjournals/joss-papers/commit/afc8a4414d32388103f16c34c9e1fbe04d8c9ffc - although it might take a while to update because of the caching @danielskatz mentioned earlier.

[marshallward]

URLs are working for me, including the typo fix. Thanks @arfon and all others for their work!

[danielskatz]

Thanks @arfon