Joss-reviews: [REVIEW]: xpecgen: A program to calculate x-ray spectra generated in tungsten anodes

[whedon]

Submitting author: @Dih5 (Guillermo Hernández)
Repository: https://github.com/Dih5/xpecgen
Version: v1.0.2
Editor: @arfon
Reviewer: @katyhuff
Archive: https://dx.doi.org/10.5281/zenodo.165764

Status

Status badge code:

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

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 questions

Conflict of interest

• [x] As the reviewer I confirm that there are no conflicts of interest for me to review this work (such as being a major contributor to the software).

  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 (v1.0.0)?

  Functionality

• [x] Installation: Does installation proceed as outlined in the documentation?

• [x] Functionality: Have the functional claims of the software been confirmed?

• [x] Performance: Have any performance claims of the software been confirmed?

  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)?
• [ ] 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

Paper PDF: 10.21105.joss.00062.pdf

• [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)?

[arfon]

@Dih5 - it doesn't look like you're actually citing the reference that you have in your paper.bib in the paper body (paper.md). Could you make sure you're explicitly citing the referenced paper (e.g. with @hernandez:2016) so we can pick up the reference list?

[Dih5]

My apologies, I forgot the cite. It is added now.

[arfon]

/ cc @openjournals/joss-reviewers - would anyone be willing to review this submission?

If you would like to review this submission then please comment on this thread so that others know you're doing a review (so as not to duplicate effort). Something as simple as :hand: I am reviewing this will suffice.

Reviewer instructions

• Please work through the checklist at the start of this issue.
• If you need any further guidance/clarification take a look at the reviewer guidelines here http://joss.theoj.org/about#reviewer_guidelines
• Please make a publication recommendation at the end of your review

Any questions, please ask for help by commenting on this issue! 🚀

[arfon]

@whedon commands

[whedon]

Here are some things you can ask me to do:

# List all of Whedon's capabilities
@whedon commands

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

# List the GitHub usernames of the JOSS editors
@whedon list editors

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

# Change editorial assignment
@whedon assign @username as editor

# Open the review issue
@whedon start review

:construction: Important :construction:

This is all quite new. Please make sure you check the top of the issue after running a @whedon command (you might also need to refresh the page to see the issue update).

[arfon]

@whedon list editors

[whedon]

Current JOSS editors:

@acabunoc
@arfon
@cMadan
@danielskatz
@jakevdp
@karthik
@katyhuff
@kyleniemeyer
@labarba
@mgymrek
@pjotrp
@tracykteal

[arfon]

@whedon assign @katyhuff as editor

[whedon]

OK, the editor is @katyhuff

[arfon]

👋 @katyhuff - any luck finding a reviewer for this submission?

[katyhuff]

After looking it over a bit I decided I'll review it myself. It's on the
to-do list for this weekend. Thanks for the bump.

On Sat, Oct 8, 2016 at 8:30 AM, Arfon Smith [email protected]
wrote:

👋 @katyhuff https://github.com/katyhuff - any luck finding a reviewer
for this submission?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/openjournals/joss-reviews/issues/62#issuecomment-252424643,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAYCq4P4r4Je9XfQPaGxh7zqV9sH_LYKks5qx5rggaJpZM4J8Bva
.

http://katyhuff.github.com

[arfon]

@whedon assign @arfon as editor

[whedon]

OK, the editor is @arfon

[arfon]

@whedon assign @katyhuff as reviewer

[whedon]

OK, the reviewer is @katyhuff

[Dih5]

Thank you both for having a look at it.
I have prepared a small update I would like to make to the software (made it python 2 compatible), so I would like to make a new release increasing the version number. Is there any problem with that change?

[arfon]

I have prepared a small update I would like to make to the software (made it python 2 compatible), so I would like to make a new release increasing the version number. Is there any problem with that change?

That's fine. What's the new release number (I'll update the thread with this information).

[Dih5]

The new release number is v1.0.1.
Thanks! :smiley:

[arfon]

The new release number is v1.0.1.

⚡️ thanks.

[katyhuff]

Review

I have a few recommendations below.

Conflict of interest

No conflict of interest

General checks

Repository

The source code for this software available at the repository url, but that url should be mentioned in the paper.

License

GPLv3.0

Version

The version given matches the GitHub release (v1.0.1)

Functionality

Installation

pip install works on my ubuntu machine. Something related to tkinter is failing on my mac. Probably something with my environment.

Functionality

I was able to run the gui as well as demo.py. The functionality is as described.

Performance

N/A

Documentation

A statement of need

The paper would be more useful if the statement of need were slightly more detailed. Who is the target audience?

Installation instructions

Installation is covered for linux/unix and windows.

Example usage

The readme could include a little more text regarding the specific buttons to press in order to get to the screenshots shown. The demo.py file is effective as a demo.

Functionality documentation:

The docstrings are fairly helpful, but not all public methods are documented with docstrings. I recommend at least one docstring for each public method in the API.

Also, there is no clickable documentation online. The doc branch has the appropriate minimal sphinx configuration to generate this html. It would be great to host that html site somewhere for users who don't want to (or cant) build the sphinx docs locally.

Automated tests

There are no automated tests of any kind. I strongly recommend adding a test suite. There are manual steps which are described so that the function of the software can be verified, but an automated set of tests would be vastly superior.

Community guidelines

This exists but is very minimal. It is limited to the statement: "If you find any bugs or have a feature request you may create an issue on GitHub. You may also considering forking and sending a pull request if you are an experienced developer." I recommend adding a contributing.md file.

Software paper

Authors

The paper lists both authors

A statement of need

The statement of need could be more detailed. It is currently as brief and minimal as possible. The likely real world applications of the tool could be more specific and the target audience is not currently identified.

References

N/A

[Dih5]

Thank you very much for the review, @katyhuff.
I have added some issues with your suggestions.

On the Mac error, please consider adding an issue detailing the problem. Although I don't own a Mac and I don't plan the application to support it, perhaps it could be addressed. It would be interesting to test if a simple hello world tkinter application is working in the system.

[katyhuff]

Excellent! Sorry it took so long to do the review.

Thanks for creating those issues. As soon as those are solved, I'd say this is more than acceptable. Regarding tests, I know that adding a test suite is a lot of effort. I'd recommend a simple pytest type framework to keep it simple. A few tests confirming the core methods would be sufficient as far as I'm concerned. It isn't necessary to acheive 100% test coverage.

It would be interesting to test if a simple hello world tkinter application is working in the system.

Sorry I wasn't more specific. It definitely isn't your package's fault. I tracked it down and seems to be known issue with a clash between anaconda and framework python: http://stackoverflow.com/questions/35593602/tk-framework-double-implementation-issue. Workaround solved it for me. So, nothing for you to worry about as its well outside of your package's scope of concern.

[Dih5]

Ok, the issues are solved now, so I think the package is ready. The changes can be tracked using the references in Dih5/xpecgen#5, Dih5/xpecgen#6, Dih5/xpecgen#7, Dih5/xpecgen#8.

I corrected a bug found thanks to unit testing, so I updated the release number to v1.0.2. The generated paper has to be updated accordingly.

Thank you @katyhuff for taking the time to review this work, I found your suggestions really valuable.

[arfon]

I corrected a bug found thanks to unit testing, so I updated the release number to v1.0.2. The generated paper has to be updated accordingly.

@Dih5 are there actual changes to the paper.md file or is it just the version that has been bumped?

[Dih5]

There are changes indeed, which were suggested by @katyhuff. The updated file can be found here.

[arfon]

Updated paper: 10.21105.joss.00062.pdf

[Dih5]

I'm afraid something went wrong with the authors names and the institutions, which are not properly displayed now.
In the last update I changed the line breaks from Windows-like(\r\n) to Unix-like(\n), perhaps that could be related.

[arfon]

OK, here's an update: 10.21105.joss.00062.pdf

Please merge this fix for the paper: https://github.com/Dih5/xpecgen/pull/13

[Dih5]

Perfect. Thanks. 😄

[arfon]

👋 @katyhuff. Would you be able to take another look at this submission to give this a 👍 /:-1:

[katyhuff]

👍

[arfon]

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

[Dih5]

Here it is:

[arfon]

@katyhuff many thanks for your review here!

@Dih5 your paper is now accepted into JOSS. Your DOI is http://dx.doi.org/10.21105/joss.00062 🎉 🚀 💥

[Dih5]

Thank you both for your work, I've learnt some nice things in this experience 😃