[REVIEW]: PyAstroPol : A Python package for the instrumental polarization analysis of the astronomical optics.

[whedon]

Submitting author: @hemanthpruthvi (Hemanth Pruthvi)
Repository: https://github.com/hemanthpruthvi/PyAstroPol
Version: v0.1.0
Editor: @pibion
Reviewer: @aquilesC, @caldarolamartin, @mwcraig
Archive: 10.5281/zenodo.4285762

⚠️ JOSS reduced service mode ⚠️

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/3d5f2d8bca0c68fa84302c3904e1638e"><img src="https://joss.theoj.org/papers/3d5f2d8bca0c68fa84302c3904e1638e/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/3d5f2d8bca0c68fa84302c3904e1638e/status.svg)](https://joss.theoj.org/papers/3d5f2d8bca0c68fa84302c3904e1638e)

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

@aquilesC & @caldarolamartin & @mwcraig, 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 @pibion 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 @aquilesC

Conflict of interest

• 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

• I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• Repository: Is the source code for this software available at the repository url?
• License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• Contribution and authorship: Has the submitting author (@hemanthpruthvi) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
• Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

• Installation: Does installation proceed as outlined in the documentation?
• Functionality: Have the functional claims of the software been confirmed?
• 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

• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• 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 functionality of the software can be verified?
• 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

• Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• State of the field: Do the authors describe how this software compares to other commonly-used packages?
• Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
• 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 @caldarolamartin

Conflict of interest

• 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

• I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• Repository: Is the source code for this software available at the repository url?
• License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• Contribution and authorship: Has the submitting author (@hemanthpruthvi) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
• Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

• Installation: Does installation proceed as outlined in the documentation?
• Functionality: Have the functional claims of the software been confirmed?
• 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

• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• 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 functionality of the software can be verified?
• 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

• Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• State of the field: Do the authors describe how this software compares to other commonly-used packages?
• Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
• 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 @mwcraig

Conflict of interest

• 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

• I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• Repository: Is the source code for this software available at the repository url?
• License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• Contribution and authorship: Has the submitting author (@hemanthpruthvi) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
• Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

• Installation: Does installation proceed as outlined in the documentation?
• Functionality: Have the functional claims of the software been confirmed?
• 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

• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• 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 functionality of the software can be verified?
• 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

• Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• State of the field: Do the authors describe how this software compares to other commonly-used packages?
• Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
• 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]

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

⚠️ JOSS reduced service mode ⚠️

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.

⭐ Important ⭐

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:

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

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

@whedon generate pdf

[whedon]

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

OK DOIs

- 10.1086/599043 is OK
- 10.1117/12.2181397 is OK
- 10.1117/12.2233176 is OK
- 10.1201/b19711 is OK
- 10.1117/1.JATIS.4.4.048002 is OK

MISSING DOIs

- None

INVALID DOIs

- None

[whedon]

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

[caldarolamartin]

Hi. I cannot modify the check list and the link to accept is expired. @pibion could you please give me a hand to solve this? Thanks

[caldarolamartin]

Hi again. I've tried to tick the boxes with out success again. I have not accepted the review formally and the the link to do so (point 2 at the top) is not working. I believe this is the reason I cannot start with the review list check. @pibion, could you please take another look?

[mwcraig]

Hi, I also cannot check the boxes....

[pibion]

@caldarolamartin, @mwcraig are you signed in to github? You should only need to (1) subscribe to the issue and (2) be signed in to edit the checkboxes.

@arfon is that correct?

[arfon]

@whedon re-invite @caldarolamartin as reviewer

[whedon]

OK, the reviewer has been re-invited.

@caldarolamartin please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations

[caldarolamartin]

Yes! now it works. Thanks!

[arfon]

@mwcraig - it looks like you need to accept the invite to the the repository here https://github.com/openjournals/joss-reviews/invitations before you can click the checkboxes.

@caldarolamartin - I've re-invited you to the repository - please accept the invite at https://github.com/openjournals/joss-reviews/invitations .

[pibion]

Thanks @arfon , shoulda checked the whedon docs :/

[caldarolamartin]

Hi! I've just posted an issue with my review comments.

[hemanthpruthvi]

Hi, I have replied to the comment.

[pibion]

@caldarolamartin just wanted to make sure you saw this update from @hemanthpruthvi .

[pibion]

@hemanthpruthvi, I think the idea of an environment file is a good one since the dependencies are somewhat sensitive. @caldarolamartin , I've always used conda environment files for this. Do you have any other recommendations?

[caldarolamartin]

Hi @pibion, I've just closed the issue I opened for my review comments. Regarding the dependencies, I've also only used conda environment files, but I know virtualenv can do the same job.

[aquilesC]

@pibion and @hemanthpruthvi , I am in the middle of the review. I strongly suggest, as @caldarolamartin points out, to create requirement files and a setup file. This will make life much easier for other users. Sorry for the self-promotion, I've written a guide on setup files here, and a very short introduction to virtual environments here that can be used as a starting point.

Even though conda environments are very handy, they are particularly well suited for isolating complete work environments. I am not completely convinced conda environment files are meant to be distributed to a wider audience unless there are stringent needs (and this is not the case for PyAstroPol that relies on widely available libraries). I believe that a conda environment.yml file may create conflicts (conda environments are not particularly flexible). I would favor a requirements.txt file that makes explicit required (minimum) versions.

[aquilesC]

I have reviewed the paper. I have noted a lot of comments that can be used by the author to improve the quality of the code and the paper but that do not impact directly the decision of whether the program is ready to be published. However, there is a (short) list of issues at the bottom of this message that must be addressed in order to make the work, in my opinion, ready to be published.

The Paper

The paper stresses too much the limitations and too little the features. The first mentioned feature says:

However, a major caveat here is that, to computeMueller matrix the electric fields of all the rays will be added coherently after propagating through the system.

Even the features are handicapped in the description of the software instead of focusing on the strengths. Of course, this is a choice of the authors, but I do think that the impact can be related to how one decides to present the work.

Something that I have missed from the paper is how the program relates to other existing packages (if any). It can be that this is the first package addressing polarization, but the author also mentions comparing to Zeemax as the ground truth. I would replace several of the stated limitations to expand on a discussion about how this program fits in the community.

Writing

The writing can be improved. There are several sentences of the form

Majority of the optical surfaces in astronomical

Instead of

The majority of the optical surfaces in astronomical

Or

The package can be described as collection of the important objects

Instead of

The package can be described as a collection of the important objects

The Code

Installation

The readme file includes a list of modules that are used, such as os, and copy which are Python built-ins, while it also adds numpy and matplotlib which are external libraries and must be installed. It also mentions mpl_toolkits which is part of matplotlib and therefore can't be installed separately.

Installation instructions must be improved. A novice user will not be able to go through them. This can be quickly solved by: adding a requirements.txt file to the root of the project, and/or creating a setup.py file specifying dependencies and versions. The latter also helps with the instruction of adding the folder to the PYTHONPATH/

Documentation

The documentation is provided as a sphinx project, but sphinx is not listed in the requirements, therefore it can't be built by a user. Even though the documentation is on-pair right now with the code, since it has no visible continuous integration tools to keep the documentation always up to date, I would suggest either leaving the _build folder out of the repo and asking people to build the docs, or using tools such as ReadTheDocs to simplify the process.

Note: The readme points towards a html file in the docs folder which, on Github, renders as the source code and not as a website. This makes it very hard to explore the documentation online and/or offline, especially when relying on sphinx-autodoc (the documentation is built from the source code of the program and therefore is not explicitly written in the docs folder).

The index page of the documentation has very little information on how to get started. It is customary to include some general information, similar to the readme file, and link to specific points for the reader to be able to crack into the program.

The code itself is very well documented. Every method and class I have checked had appropriate docstrings, so compliments to the author for taking the time to do such a thorough work. That really pays off for anybody trying to learn from the code and, potentially, contribute to it.

Quality of the Code

The program performs the tasks that it needs to perform. However, inspection of the code leaves several rooms of improvement that relate to standards (but not requirements) of how Python programmers work.

For example, using from PyAstroPol import * is highly discourage, since this is prone to name clashes and makes it very hard to understand what is going to be available to the developer. This is done through the code everywhere, for example in the __init__ file every module is imported with import *. And this propagates to the example on the Readme, where matplotlib is never imported but the code still works. This behavior is also visible within the program files. For example, material.py uses numpy, but numpy is never imported.

Another topic is the non-adherence to PEP8. Again, these are guidelines, but that can ensure the long-life of a project by lowering the barrier to future contributors. I believe all methods use lowerCamelCase, but attributes of classes don't follow a regular pattern. For example, in coating.py there is self.Wavelength and self.iRI.

Examples

The examples are a great starting place. I think they are the best addition to the code since they very quickly allow a new user to get the gist of the program and start adding small modifications to it.

However, all the examples run into a RuntimeWarning because of a division by 0. This raises the concern of whether the code is actually performing the right calculation or if there are problems that may have an impact on the obtained values. I have no way of verifying the accuracy claims:

Results of the code have been verified against previous works (Pruthvi, Krishnappa, Belur, &Kadagattor (2018)), and ZEMAX© OpticStudio – a popular commercial software.

Materials Licenses (discussion)

The code re-distributes material properties as csv files under an MIT license. Many of this materials are taken from PhysRevB which is not necessarily compatible with the chosen license. I am not 100% sure whether there is a license incompatibility or whether people is free to redistribute data without consent. Any advice from @pibion @caldarolamartin or @mwcraig ?

Conclusions

The paper and the code are ready to be published provided that the author performs the following:

• Improve the writing of the paper
• Clear placement of the current program in the field, i.e. what other packages exist and how this one relates to them
• Clearly explain how to install the program. Ideally making dependencies (and not built-in python modules) version-specific such as what a requirements.txt file would provide
• Explain why having a RuntimeWarning pointing towards a division by zero still generates the proper results. Ideally, this warning should be handled by the program so the end user does not need to worry about it.

[hemanthpruthvi]

@aquilesC Thank you very much for the thorough review and the valuable comments, I am working on addressing them. Meanwhile, I want to quickly address the issue regarding the Material Licenses. If I understand correctly, there are two issues.

1. Can the Material files be re-distributed, according to their existing licensing?
2. If so, can they be re-distributed under MIT license?

For issue 1 :
Albeit the information listed in the Material files originated in various publications, the raw files themselves are directly downloaded from https://refractiveindex.info/. The sources of the data are also listed there. The website's database is public domain via CC0 1.0 license i.e., whatever data hosted there is free to be used/distributed without seeking any consent.

For issue 2 :
If re-distributing under MIT license is an issue, I propose either of two ways for resolution, whichever is plausible.

1. Having two licenses : MIT -- for the code, and CC0 1.0 -- for the database, or
2. Exclude the database from the scope of the existing license, and simply state --
   "XXX data is downloaded from YYY, hosted under ZZZ license"

[aquilesC]

@hemanthpruthvi I have seen the license of refractiveindex.info, but I am not sure whether the author is actually entitled to re-distribute the data under a CC0 license. It would, for example, eliminate the citation to the original work with the real measurements. The sources of the website are papers not published under open access and I haven't seen information on APS regarding the distribution of datasets. I know, for example, that Comsol distributes some material properties but more recent measurements can't always be built-into the program, therefore they point you in the direction of how to get the dataset. In any case, this is a out of my depth regarding my understanding of copyright. I think the polite (and perhaps correct) thing to do is to add, in the materials folder, the citation of where the indexes come from (not the website, but the original work) since this is the best way of having a reproducible workflow in the future. You could add a Readme file directly in that folder.

[hemanthpruthvi]

@aquilesC I understand and I agree. Accordingly, I have added a Readme file in the materials folder that refers to original sources.

[hemanthpruthvi]

@whedon generate pdf

[whedon]

OK. 10.5281/zenodo.4285762 is the archive.

[pibion]

@whedon accept

[whedon]

Attempting dry run of processing paper acceptance...

[whedon]

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

OK DOIs

- 10.1086/599043 is OK
- 10.1117/12.2181397 is OK
- 10.1117/12.2233176 is OK
- 10.1201/b19711 is OK
- 10.1117/1.JATIS.4.4.048002 is OK
- 10.1007/BF00157317 is OK
- 10.1051/0004-6361:20052935 is OK

MISSING DOIs

- None

INVALID DOIs

- None

[whedon]

👋 @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof 👉 openjournals/joss-papers#1933

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#1933, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true

[pibion]

@whedon accept deposit=true

[whedon]

I'm sorry @pibion, I'm afraid I can't do that. That's something only editor-in-chiefs are allowed to do.

[arfon]

@hemanthpruthvi - I made a few formatting fixes in this PR (mostly related to citations): hemanthpruthvi/PyAstroPol#2

Once you've merged this, I will go ahead and publish.

[hemanthpruthvi]

@arfon Done.

[arfon]

@whedon accept

[whedon]

Attempting dry run of processing paper acceptance...

[whedon]

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

OK DOIs

- 10.1086/599043 is OK
- 10.1117/12.2181397 is OK
- 10.1117/12.2233176 is OK
- 10.1201/b19711 is OK
- 10.1117/1.JATIS.4.4.048002 is OK
- 10.1007/BF00157317 is OK
- 10.1051/0004-6361:20052935 is OK

MISSING DOIs

- None

INVALID DOIs

- None

[whedon]

👋 @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof 👉 openjournals/joss-papers#1935

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#1935, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true

[arfon]

@whedon accept deposit=true

[whedon]

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

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

0. Check final PDF and Crossref metadata that was deposited 👉 Creating pull request for 10.21105.joss.02693 joss-papers#1936
1. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.02693
2. If everything looks good, then close this review issue.
3. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? Notify your editorial technical team...

[kyleniemeyer]

Congrats @hemanthpruthvi on your article's publication in JOSS!

Many thanks to @aquilesC, @caldarolamartin, and @mwcraig for reviewing this, and @pibion for editing.

[whedon]

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](https://joss.theoj.org/papers/10.21105/joss.02693/status.svg)](https://doi.org/10.21105/joss.02693)

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

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

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: https://joss.theoj.org/reviewer-signup.html
• Making a small donation to support our running costs here: https://numfocus.org/donate-to-joss