shreyasbapat / python-oifits Goto Github PK

One of the checklist items is "Installation instructions: Is there a clearly-stated list of dependencies?". I don't see that list in the installation instructions.

In addition, the dev install instructions say einsteinpy in a couple of places instead of oifits.

openjournals/joss-reviews#1743

The example usage at the moment includes a single, very brief, jupyter notebook. Having jupyter notebooks as the primary example documentation is fine (and even a single notebook is fine) but there needs to be more extensive explanation in the notebook. For example, an example that included loading some data from the EHT data site (Assuming there is one) and extracting some physical information from it would be much more helpful.

openjournals/joss-reviews#1743

Hi @shreyasbapat, @arnavbhavsar and @ananyas2501,

I'm one of the reviewers for your submission to JOSS, openjournals/joss-reviews#1743.

I appreciate the effort you have put into documenting what you have done, and writing some tests.

However, I think there is a major issue that needs to be resolved before the paper could be accepted.

Roughly 90% of the code in this repository is taken directly from @pboley's oifits at https://github.com/pboley/oifits.

In several places some reformatting to be closer to PEP8 has been done, and the code has been updated to use astropy.io.fits instead of pyfits (though @pboley's code also uses astropy.io.fits if pyfits is not installed), and the high-level class structure is different. At the end of this issue I provide a few of examples of overlap.

There are a few problem with that:

1. From a software license point of view, @pboley removed the license from the code (it had been BSD 3-clause, I think?) in pboley/oifits@550aca8. That means (whether that was their intention or not) that you need their permission to reuse the code, and definitely need their permission to include their code in your code.
2. From the point of view of the paper, @pboley ought to be included as an author and arguably ought to be the primary author, given the extent of their code authorship.
3. In the event that @pboley permits use of the code after you request it but declines to be involved in the paper then you would need to much more prominently acknowledge his prior work in both each of the code files and in the paper.

There are a couple other things I'll address briefly in other issues.

Examples of code re-use in python-oifits from oifits

I've included one example from each of the four files in the python-oifits source (excluding the tests). Each of those four files contains part of the file oifits.py in the oifits source:

1. In https://github.com/shreyasbapat/python-oifits/blob/master/src/oifits/data.py, compare the code starting at https://github.com/shreyasbapat/python-oifits/blob/master/src/oifits/data.py#L20 with the code at https://github.com/pboley/oifits/blob/master/oifits.py#L569
2. In https://github.com/shreyasbapat/python-oifits/blob/master/src/oifits/dataclasses.py, compare each of the class definitions with those in oifits, e.g. compare the definition of OI_TARGET in https://github.com/shreyasbapat/python-oifits/blob/master/src/oifits/dataclasses.py#L14 to the definition of the same class in https://github.com/pboley/oifits/blob/master/oifits.py#L165. The same comparison can be done for OI_WAVELENGTH, OI_VIS, OI_VIS2, OI_T3, OI_STATION, and OI_ARRAY.
3. In https://github.com/shreyasbapat/python-oifits/blob/master/src/oifits/read.py, compare the definition of process at https://github.com/shreyasbapat/python-oifits/blob/master/src/oifits/read.py#L39 to the definition of open in https://github.com/pboley/oifits/blob/master/oifits.py#L1285.
4. In https://github.com/shreyasbapat/python-oifits/blob/master/src/oifits/utils.py, compare the definition of _plurals at https://github.com/shreyasbapat/python-oifits/blob/master/src/oifits/utils.py#L6 with the definition of _plurals at https://github.com/pboley/oifits/blob/master/oifits.py#L93. The subsequent definitions of _array_eq and _angpoint are also essentially the same.

ping the editor, @arfon....

Looking through the documentation for the package I noticed that although there is a page for API documentation at https://oifits.readthedocs.io/en/latest/api/index.html there is no API documentation at that page.

Part of openjournals/joss-reviews#1743