Joss-reviews: [REVIEW]: surfinpy: A Surface Phase Diagram Generator

[whedon]

Submitting author: @symmy596 (Adam Symington)
Repository: https://github.com/symmy596/SurfinPy
Version: v1.0.1
Editor: @labarba
Reviewer: @bocklund, @mkhorton
Archive: 10.5281/zenodo.2573647

Status

Status badge code:

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

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

@bocklund & @mkhorton, 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 @labarba know.

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

Review checklist for @bocklund

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: v1.0.1
• [x] Authorship: Has the submitting author (@symmy596) 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 @mkhorton

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: v1.0.1
• [x] Authorship: Has the submitting author (@symmy596) 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. @bocklund, 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:

[labarba]

👋 @symmy596 — Please check your references list, aided by the bot's suggestions in the pre-review issue: https://github.com/openjournals/joss-reviews/issues/1202#issuecomment-457943767

[labarba]

@bocklund, @mkhorton — Thank you for agreeing to review for JOSS! This is where the action happens: work your way through the review checklist, feel free to ask questions or post comments here, and also open issues in the submission repository as needed. Godspeed!

[symmy596]

Thank you @labarba for the speed that you have gone through this so far. The references should now be updated and the doi's added.

[labarba]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

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

[bocklund]

@symmy596 looks great! The API and theory is well documented and matplotlib plots look especially nice. I have a few comments:

1. Documentation - Installation instructions: The requirements found in requirements.txt should be added to setup.py, otherwise users installing from pip will not get all the requirements.
2. Software Paper - References: looks like reference Reuter2003 did not get correctly inserted into the proof.
3. Functionality - Functionality: Please run the example notebooks and put them on GitHub so the outputs can be rendered. This lets users know that they are getting the right answers when they run the code. The tutorials seem to run fine for me, just not sure if I'm seeing the expected output.
4. HTML for logo is broken for me on Readthedocs
5. Documentation - Community guidelines: Unclear how users should seek support.

Some suggestions (not blocking acceptance, in my opinion)

1. It looks like your tags on GitHub are not updated to the latest release version, but you have used the tags in the past. Something that is nice for contributors and future code-owners is to document the release process steps. Here's an example of one for ESPEI, which I use every release
2. Readme badges are not working for me on PyPI and the links are not rendered (PyPI uses reStructuredText)

[symmy596]

@bocklund Thank you for taking the time to review the project. I think I have addressed your comments

1) I have added the required packages to setup.py under install_requires
2) Reuter2003 was missing the @ and thus was not being added. I have fixed this.
3) Example notebooks have been run and uploaded.
4) HTML logo is now fixed.
5) I have added contact information under the Contributing section of the README. A user now as the option to email me directly, open an issue on the issue tracker, or discuss their question on gitter.

PyPi - I have added - long_description_content_type="text/markdown” to the setup.py. This allows markdown to be rendered on PyPi. An example of a project where this has been done can be found here - https://github.com/bjmorgan/bsym
To the best of my knowledge, a new release is needed to update the Readme on PyPi so this will be fixed when I create the next release. I think it is probably best if I wait for the review from @mkhorton before releasing another version.

If I have missed anything or misunderstood anything then please get back to me.

Question for @labarba
I have added a figure to the paper that illustrates an example output from the code because I think it makes it much clearer what the code is all about. I am not sure I am allowed to edit a paper post submission so if I am not then I am very sorry and I will revert back to the previous version.

Thanks

[symmy596]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

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

[labarba]

It's fine to add a figure if you decided that improves the paper.

[mkhorton]

I have a minor visual glitch with example 5 script, where the Wulff shape is off-centered/partially cut off and the legend also cut off (using default requirements from the GitHub repo, Python 3.7, macOS).

Edit: apologies, realized it would have been better to create an issue for this.

[symmy596]

@mkhorton - This sounds like it is a pymatgen issue opposed to a surfinpy issue. surfinpy is used to calculate the surface energies and then these are used to generate the Wulff plot with pymatgen. I suppose the first thing to check is which version of pymatgen are you using? I have been using Version 2018.11.30

[mkhorton]

That’s entirely possible, I’ll investigate further. Thanks.

On Tue, Feb 12, 2019 at 02:58, Adam Symington [e-mail]
wrote:

@mkhorton https://github.com/mkhorton - This sounds like it is a
pymatgen issue opposed to a surfinpy issue. surfinpy is used to calculate
the surface energies and then these are used to generate the Wulff plot
with pymatgen. I suppose the first thing to check is which version of
pymatgen are you using? I have been using Version 2018.11.30

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

[symmy596]

@bocklund - Would you like me to comment the specific commits where the changes where made, sorry I realise that I probably should have done this anyway.
Cheers

[bocklund]

@symmy596 You comments from before were plenty, thanks for pinging me again.

@labarba I've completed the checklist and everything looks good to me.

[mkhorton]

I'm taking one last look too, but looks good to me as well, will update my checklist shortly (@symmy596 you were correct that the previous issue I mentioned is with pymatgen/matplotlib, not with surfinpy)

[labarba]

@bocklund, @mkhorton — Thank you, both. I'm going to ask for a clear statement from you, when you're done, that you recommend publication.

[mkhorton]

I'm happy to recommend publication, looks very useful! Just checked off the remaining items.

The sole outstanding issue is just that the GitHub release (0.7) doesn't match the PyPi version (0.8) which also needs to be updated to correct the Markdown formatting, but as I understand it this is being addressed (also the docs are at v0.4?). I reviewed both the PyPi version + cloned from source.

I would encourage including some of the gallery figures in the GitHub readme, since they're very compelling.

[labarba]

Regarding the version, @symmy596, after all revisions are done, and you issue a new tagged release, I can update the version number associated to the paper with a whedon command here.

[symmy596]

Hi

@mkhorton I have fixed the docs version number. I will add a couple of figures to the readme, this is a good suggestion. Thank you @bocklund and @mkhorton for the time that you put in to the review and for your kind comments.

@labarba - I have addressed all of the comments and updated the PyPi release to version 1.0 and I have fixed the PyPi readme - https://pypi.org/project/surfinpy/
I think that should be everything complete now.

Thank you for your help during this review.

[bocklund]

@labarba I recommend surfinpy for publication in JOSS

[labarba]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

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

[symmy596]

@mkhorton unless you have any issues can you update the functionality and version tags in the review so that it is complete.
Cheers

[labarba]

Minor editorial fixes on paper:

• Thus understanding the state of surface is crucial >> Thus, understanding the state of a surface is crucial [missing comma and article]
• using a well established approach >> well-established (hyphen)
• Once the energy is known under different constants the phase >> comma after "constants"
• and thus a phase diagram >> commas before and after this remark
• considering surface defects e.g. vacancies >> commas before and after "e.g." ... same next line
• contructed >> constructed
• spcies >> species
• e.g. oxygen >> comma after "e.g." and after "oxygen"
• method of Marmier (Marmier & Parker, 2004) >> method of Marmier & Parker (2004) [*]
• generating a surface phase diagrams from DFT data >> delete article "a" _or_ remove the plural in "diagrams"
• in Molinary _et al_ (Molinari et al., 2012) >> in Molinary et al. (2012) [*]
• Marmier _et al_ >> same above [*]
• temperature vs pressure >> vs. (with dot)
• Figure 1: This figure illustrates an example phase diagram >> Figure 1: An example phase diagram (cut the clutter)
• physicist and chemists >> physicists
• jupyter notebooks >> capitalize Jupyter

[*] For the syntax to obtain brackets in the right places, see: https://rmarkdown.rstudio.com/authoring_bibliographies_and_citations.html

[mkhorton]

@symmy596 thought I had! Done now

[symmy596]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

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

[symmy596]

Sorry, I missed one. The following is the correct update.

[symmy596]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

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

[labarba]

On line 63:

It contains two core modules for generating surface phase diagrams using both the methods employed in Molinari *et al* [-@Molinari2012] and Marmier *et al* [-@Marmier2004].

We don't want a manual *et al*, but the proper use of markdown citation that gets the Author (year) format.

[labarba]

I think you want an "in-text citation" : @smith04 says blah.

[symmy596]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

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

[symmy596]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

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

[symmy596]

@labarba I think I have fixed it.

Cheers

[labarba]

OK, @symmy596 — Now, you should make a new tagged release, report the version number here, then make an archive in a service like Zenodo, and reporte the DOI here. Cheers!

[symmy596]

@labarba Thank you

Tagged release has been created for version 1.0.1
Archive on Zenodo created and the DOI is - 10.5281/zenodo.2573646

Cheers

[labarba]

@whedon set v1.0.1 as version

[whedon]

OK. v1.0.1 is the version.

[labarba]

@symmy596 Please edit the Zenodo metadata (title and authors) to match the JOSS paper. Thanks!

[symmy596]

@labarba - Is it obvious that this is my first time lol.

Here is the doi - 10.5281/zenodo.2573647

Cheers

[labarba]

Oh dear, you got a new DOI?

[labarba]

I'm confused ... we have TWO Zenodo deposits now?

[labarba]

It looks like 10.5281/zenodo.2573647 is as newer version of the previous deposit (10.5281/zenodo.2573646) — I hadn't seen the DOI change with an editing of the metadata before. Not sure what happened.

[labarba]

@whedon set 10.5281/zenodo.2573647 as archive

[whedon]

OK. 10.5281/zenodo.2573647 is the archive.

[symmy596]

@labarba I have no idea how I did that, it has been a long day. Sorry, is it ok now?

Cheers

[labarba]

@whedon accept

[whedon]

Attempting dry run of processing paper acceptance...

[whedon]

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

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/509, 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]

```Reference check summary:

OK DOIs

• http://doi.org/10.1021/acs.chemmater.8b00649 is OK
• http://doi.org/10.1021/jp300576b is OK
• http://doi.org/10.1021/acs.jpcc.6b10986 is OK
• http://doi.org/10.1103/PhysRevLett.90.046103 is OK
• http://doi.org/10.1103/PhysRevB.69.115409 is OK

MISSING DOIs

• None

INVALID DOIs

• None
  ```

[labarba]

@whedon accept deposit=true

[whedon]

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

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

[labarba]

Congratulations, @symmy596, your JOSS paper is now published!

Big thank you to the reviewers, @bocklund, @mkhorton — we couldn't do this without you 🙏

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

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

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

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://www.flipcause.com/secure/cause_pdetails/Mjk3Nzk=