[REVIEW]: coxeter: A Python package for working with Shapes

whedon commented 3 years ago

Submitting author: @vyasr (Vyas Ramasubramani) Repository: https://github.com/glotzerlab/coxeter Version: v0.6.1 Editor: @jni Reviewers: @wolfv, @evetion Archive: 10.5281/zenodo.5106336

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

Status badge code:

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

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

@tmaric, @wolfv please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

Make sure you're logged in to your GitHub account
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 @jni know.

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Review checklist for @evetion

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

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] Contribution and authorship: Has the submitting author (@vyasr) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

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 functionality 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] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of Need' that clearly states what problems the software is designed to solve and who the target audience is?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

Review checklist for @wolfv

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

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] Contribution and authorship: Has the submitting author (@vyasr) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

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 functionality 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] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of Need' that clearly states what problems the software is designed to solve and who the target audience is?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @tmaric it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

: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:

Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

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

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf

whedon commented 3 years ago

Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.85 s (87.0 files/s, 66090.2 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
GLSL                             1              0              0          23239
JSON                             2              0              0          20304
Python                          42           1858           2938           5383
reStructuredText                13            191            103            408
Jupyter Notebook                 2              0            668            401
TeX                              2              7              0            294
Markdown                         6             55              0            169
YAML                             3             22              4            133
DOS Batch                        1              8              1             26
TOML                             1              0              0             10
make                             1              4              7              9
-------------------------------------------------------------------------------
SUM:                            74           2145           3721          50376
-------------------------------------------------------------------------------

Statistical information for the repository '2258d70b8847a7535d2b1e8a' was
gathered on 2021/03/11.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Bradley Dice                   110          2820           1576            3.27
Brandon Butler                  17          2867           3249            4.55
Bryan VanSaders                 23          1565           1055            1.95
Corwin Kerr                      2             8              8            0.01
Eric Harper                      1          1825              0            1.36
Jens Glaser                      1         44855              0           33.39
M. Eric Irrgang                  2           863          44857           34.04
Tim Moore                        4            42             26            0.05
Tobias Dwyer                    40          1825            289            1.57
Tommy Waltmann                  17           334            247            0.43
Vyas Ramasubramani             434         15298          10379           19.12
Will Zygmunt                     1           129            200            0.24

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Bradley Dice               1213           43.0          7.7                9.98
Brandon Butler                1            0.0         18.2                0.00
Bryan VanSaders               1            0.1         53.8                0.00
Corwin Kerr                   6           75.0          2.1                0.00
Tim Moore                     1            2.4         13.3              100.00
Tobias Dwyer                491           26.9          4.5                3.87
Tommy Waltmann              153           45.8          1.1               14.38
Vyas Ramasubramani         8313           54.3          7.5               13.70

whedon commented 3 years ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1126/science.1220869 is OK
- 10.1016/j.commatsci.2019.109363 is OK
- 10.1103/PhysRevX.4.011024 is OK
- 10.1063/5.0019735 is OK
- 10.1038/nmat1949 is OK
- 10.1080/00268970601075238 is OK
- 10.1080/2151237X.2006.10129220 is OK
- 10.1021/acs.langmuir.7b02384 is OK
- 10.21105/joss.00787 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1038/s41592-019-0686-2 is OK
- 10.1039/C9SM00887J is OK
- 10.1109/MCSE.2018.05329813 is OK
- 10.1063/1.5052551 is OK
- 10.1103/PhysRevLett.107.135701 is OK
- 10.1073/pnas.1415467112 is OK

MISSING DOIs

- None

INVALID DOIs

- None

whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

jni commented 3 years ago

@whedon add @wolfv as reviewer

whedon commented 3 years ago

OK, @wolfv is now a reviewer

jni commented 3 years ago

@tmaric @wolfv, thank you again for volunteering to review, much appreciated! :pray: In case this is your first time here, JOSS reviews have a nice, guided structure described in our review guidelines, review criteria, and the checklist above. Reviews are collaborative, much like code review. If you have any questions about the process, don't hesitate to ask here.

Thanks again!

whedon commented 3 years ago

:wave: @tmaric, please update us on how your review is going (this is an automated reminder).

jni commented 3 years ago

Hi @tmaric and @wolfv, just a kind ping to make sure this doesn't fall completely by the wayside. All good if you haven't had time yet. 😊

jni commented 3 years ago

Kind ping again @tmaric and @wolfv. Please also let us know if your situation has changed and we need to find different reviewers. 🙏

tmaric commented 3 years ago

Hi, sorry, chaos on my side, I'm on it.

wolfv commented 3 years ago

Hi, so finally found some time to read the paper and take a look at the docs + code.

I think this is excellent work!

I have some comments to the documentation of this project:

it would be great to see some 'real-life' example use (maybe in a jupyter notebook, with visible plots of the different shapes, too)
I think the docs are very thorough, but a tutorial or some other high-level introduction seems to be missing (might go hand-in-hand with the real-life example mentioned above).

The paper reads great and the authors demonstrate a clear need for a software like this. Does this make sense as a review? It's my first time doing a JOSS review, so let me know if I need to add more review notes.

jni commented 3 years ago

Yes, good tips @wolfv and thank you for taking the time to look over everything!

The only question is whether you would suggest this as feedback for the authors but still recommend acceptance, or whether you see a tutorial/real-life example as a pre-requisite before publication. I can see arguments either way.

vyasr commented 3 years ago

Yes thanks for the review @wolfv. Whether or not you think this is a prerequisite for publication, a tutorial+examples is definitely something I'll work to add. What would you think about publishing the notebooks used to generate the components of the paper figure as a set of representative "real-life" examples? Each panel in the figure was made with a separate notebook that I could directly add (probably most appropriate after the paper is published and I can post slightly modified versions to avoid any copyright issues).

jni commented 3 years ago

@vyasr since you are the author you can choose whatever copyright you want. If you use CC-BY/CC0 for your docs and refer to them in the paper, everything should be fine. We don't publish papers that have been published in other peer-reviewed venues, but basing the paper on the docs and vice versa is A-ok.

vyasr commented 3 years ago

Conversely, since the paper will be published under CC-BY 4.0 (at least the last one that I published here was), it should also be fine to go in the opposite direction and cite the paper in the docs after it's been published, correct? Assuming @wolfv is fine to publish the paper without these examples already in place, I'll probably wait until this review is done then do a an update to the docs both to add references to this paper and add the new examples.

jni commented 3 years ago

@tmaric would you have some time in the coming days to dedicate to this review? Otherwise if you have some sense of when you will have time, that would be useful to hear too. 😊 Thank you! 🙏

jni commented 3 years ago

@vyasr I'm worried that circumstances may have made it more difficult for @tmaric to go over a review. Do you perhaps have other suggestions for reviewers? Usernames only (no @), and then I can ping them separately.

In the meantime, I might suggest updating the docs as suggested by @wolfv, as it will only strengthen the submission for the next reviewer. 😊

vyasr commented 3 years ago

Yes, I can update the docs and provide you new reviewers as soon as I'm able!

vyasr commented 3 years ago

Other possible reviewers are hugoledoux (I know he's also an editor, so not sure what his time is like), vissarion, and evetion.

I'm working on adding an edited version of the paper figures into the docs now, I'll update when that's done!

vyasr commented 3 years ago

I've opened a PR adding simplified examples based on the figures as per @wolfv's suggestion!

vyasr commented 3 years ago

@jni any luck finding a second reviewer? No rush, just let me know if you need me to recommend additional names.

jni commented 3 years ago

@vyasr apologies, I have been overwhelmed by other work — I will make another push here. Thank you for your patience! 🙏

vyasr commented 3 years ago

No problem, whenever you can fit this into your schedule is fine with me! I appreciate your efforts.

jni commented 3 years ago

@whedon add @evetion as reviewer

whedon commented 3 years ago

OK, @evetion is now a reviewer

jni commented 3 years ago

@whedon remove @tmaric as reviewer

whedon commented 3 years ago

OK, @tmaric is no longer a reviewer

jni commented 3 years ago

@evetion thank you so much for agreeing to review this submission! 🙏 You should have got an invitation to join the repo, which you can check at https://github.com/openjournals/joss-reviews/invitations. Accepting the invitation will allow you to tick off boxes in the checklist at the top of this issue. Please let me know if you don't have access and I'll reinvite you.

JOSS reviews happen in public. You can add comments to this issue to raise questions with the authors, or create issues on the original software repo — whichever you find more appropriate for a particular discussion.

Please let me know if you have any more questions about the process!

evetion commented 3 years ago

@vyasr I think this is a very nice submission/paper/package. My domain knowledge is in shapes, but often with macro applications, while this is mostly on a micro scale.

blocking questions

Authorship: What's the contribution of Sharon C. Glotzer? See this for guidelines.
There's no statement of need in the documentation. See this for guidelines. You could copy it from the paper.

non-blocking

Documentation link is hard to find. I made an issue.
The documentation could use some intermediate level of explanations, like a statement of need, but also to explain that while the shapes are generic, the provided functions are mostly for the sciences on a micro-scale (correct me if I'm wrong). Also made an issue for it.

PS How did you find me as a possible reviewer?

vyasr commented 3 years ago

@evetion thanks for the review! Apologies for the slow response, I'm in the middle of a few things but will get back to you ASAP. It looks like one of my coauthors already helped to start addressing your issues.

Regarding how I found you, you're on the list of potential JOSS reviewers. I know that I added myself at one point at someone's request, possibly around when I first submitted a paper to JOSS.

evetion commented 3 years ago

@vyasr The recent activity on the issues over at coxeter is great, but note that most of those are nice to haves and not required for joss. In case this wasn't clear, for this review to succeed, there are two open blocking questions in my comment above, namely the statement of need in the documentation and the question of authorship.

vyasr commented 3 years ago

@evetion thanks for the ping on that. I'd appreciate any feedback from you on my work on the non-blocking items as well, but regarding the blockers:

As I'm sure you noticed, Sharon Glotzer has not contributed any code to coxeter, but she has been involved in all of the research that led to its creation and helped guide the creation of prototypes for many features that eventually made their way into the package. Some concrete examples of those features that eventually found their way into the package:
- Calculating diffraction patterns on highly anisotropic building blocks was a major focus of a dissertation published in our group a number of years ago. Sharon was involved in determining the necessity and the means for incorporating anisotropic form factors into this calculation, which led to the initial prototypes for calculating form factors that I worked into for coxeter.
- Some of the research that Sharon and I worked on involves studying the dynamics of anisotropic particles. Those types of simulations require precise calculations of the inertia tensor of a shape in order to reproduce accurate dynamics, and we found that inaccurate calculations were leading to incorrect results. That drove the implementation of the more robust triangulation and inertia tensor calculations in coxeter.
- Another research area that Sharon has recently been pushing is in theoretical approaches to anisotropic particles based on perturbation theories. Many of those perturbation theories rely on treating shapes relative to their inspheres/incircles, which means we needed robust ways to estimate the insphere radii of shapes as well as the relative dilation at a particular point on the surface. That was another calculation that ended up in coxeter.
Some of my recent activity was targeted at your request for a more visible statement of need, specifically the changes to the README. Do you think this is enough information, or would you prefer that I inserted more of the statement of need from the paper draft directly into the paper?

evetion commented 3 years ago

@vyasr Thanks for the explanation. The recent documentation is a good improvement and with the assumption you'll merge the PR, I've checked the last review boxes.

bdice commented 3 years ago

@evetion Your suggestions were very helpful! Thanks again. 😊

jni commented 3 years ago

@evetion fantastic! Thank you for that review! Can you confirm that you are ready for the paper to be accepted then?

evetion commented 3 years ago

@evetion fantastic! Thank you for that review! Can you confirm that you are ready for the paper to be accepted then?

@jni Yep, I confirm we can accept this paper. 🎉

vyasr commented 3 years ago

Great! I just merged that last PR.

jni commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

jni commented 3 years ago

@whedon check references

whedon commented 3 years ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1126/science.1220869 is OK
- 10.1016/j.commatsci.2019.109363 is OK
- 10.1103/PhysRevX.4.011024 is OK
- 10.1063/5.0019735 is OK
- 10.1038/nmat1949 is OK
- 10.1080/00268970601075238 is OK
- 10.1080/2151237X.2006.10129220 is OK
- 10.1021/acs.langmuir.7b02384 is OK
- 10.21105/joss.00787 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1038/s41592-019-0686-2 is OK
- 10.1039/C9SM00887J is OK
- 10.1109/MCSE.2018.05329813 is OK
- 10.1063/1.5052551 is OK
- 10.1103/PhysRevLett.107.135701 is OK
- 10.1073/pnas.1415467112 is OK

MISSING DOIs

- None

INVALID DOIs

- None

jni commented 3 years ago

@vyasr Things are looking good! Could you please:

make a new tagged release on github
make a new Zenodo archive (ensure that the title matches the paper title)
report the DOI and version number here

Thank you!

vyasr commented 3 years ago

@jni Here's the new tagged release on Github and the link for PyPI. The Zenodo archive is here for the current version, which is now 0.6.0, and the DOI is 10.5281/zenodo.5102768. Thanks!

vyasr commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

vyasr commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

vyasr commented 3 years ago

@jni quick heads up, my coauthor @bdice noticed a very minor typo in one of the citations that I just fixed on our main branch. Those changes came after the release and the Zenodo archive, hope that's not an issue.

jni commented 3 years ago

@vyasr it's not an issue in that it's easily fixable, but unfortunately we will need you to tag/release/archive again, sorry about that! 0.6.1? :grimacing:

openjournals / joss-reviews