olebole / python-cpl

Python bindings for CPL recipes
http://pypi.python.org/pypi/python-cpl
GNU General Public License v2.0
10 stars 5 forks source link

Astropy Affiliated Package Review #12

Closed astrofrog closed 6 years ago

astrofrog commented 6 years ago

This package has been reviewed by the Astropy coordination committee for inclusion in the Astropy affiliated package ecosystem.

We have adopted a review process for affiliated package that includes assigning quantitative ‘scores’ (red/orange/green) for different categories of review. You can read up more about this process here. (This document, currently in Google Docs, will be moved to the documentation in the near future.) For each of the categories below we have listed the score and have included some comments when the score is not green.

Functionality/ScopeSpecialized%20package
No further comments
Integration with Astropy ecosystemPartial
It would be worth seeing if this can make use of ccdproc or the CCDData class in the core astropy package.
DocumentationPartial
The documentation that is present currently is good but could be expanded. It might help readability to also not have all the documentation on a single page.
TestingPartial
The test coverage appears to be 76% but the 'oca' directory is not included in that calculation.
Development statusFunctional%20but%20low%20activity
No further comments
Python 3 compatibilityGood
No further comments

Summary/Decision: This package meets the review criteria for affiliated packages, so we are happy to confirm that we'll be listing your package as an affiliated package. Keep up the good work, and we encourage you to improve on the areas above that weren't “green” yet.

If you agree with the above review, please feel free to close this issue. If you have any follow-up questions or disagree with any of the comments above, leave a comment and we can discuss it here. At any point in future you can request a re-review of the package if you believe any of the scores should be updated - contact the coordination committee, and we’ll do a new review. Note that we are in the process of redesigning the http://affiliated.astropy.org page to show these scores (but not the comments). Finally, please keep the title of this issue as-is (“Astropy Affiliated Package Review”) to make it easy to search for affiliated package reviews in future.

olebole commented 6 years ago

Thank you very much for the review! I am fine with it, and will close it now. I have just two remarks:

The use of ccdproc of CCDData in python-cpl is probably not useful: python-cpl is completely agnostic with respect to the pipeline recipes, and depends on them. The exact structure of the underlying data (aside from being FITS files) is completely defined by the ESO recipes, and varying largely between the instruments: for example the FORS pipeline is independent from MUSE. Even the basic CCD reduction (bias, dark, flat) is different, due to different goals of the instruments and different CCDs and readout. It does not make sense to re-implement (and maintain!) that within an astropy affiliated package. Instead it is recommended to use the according recipes from the ESO pipelines for that (which is the goal of python-cpl).

Another remark is about the exclusion of the oca subdir from testing: oca was work in progress for some time, but its development is stalled, and I may even remove it from the repository. It never reached a state where it can be useful, and was never included in the distribution.