RiceMunk / omnifit

This is a package for doing ice spectroscopy fitting of interstellar ices.
https://ricemunk.github.io/omnifit/
6 stars 5 forks source link

Astropy Affiliated Package Review #35

Open astrofrog opened 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
In our previous review, we had mentioned that there are a number of places where astropy could be used but isn't - for example astropy.modeling doesn't appear to be used, and units are used only sometimes but not consistently. Once the new specutils package is ready (providing a common spectrum base class), we will need to re-assess the integration.
DocumentationPartial
The documentation is quite short at the moment and could do with a better narrative and more examples.
TestingPartial
There are a number of tests but the continuous integration (Travis) doesn't appear to be running correctly.
Development statusFunctional%20but%20low%20activity
No further comments
Python 3 compatibilityPartial
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. However, note that a number of areas were 'orange', so we encourage you to improve on these areas.

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.

dhomeier commented 2 years ago

Thank you again for your ongoing efforts to keep this package up to date @RiceMunk. As mentioned the requirements for becoming an affiliated package include, since last year, support of Python 3. For existing affiliated packages that do not yet meet this requirement, there is a 6 month period to add Python 3 compatibility in order to retain their affiliated status. Since the code in #43 already fully supports Python 3.10, I am confident that the package will be able to meet this goal by July.

RiceMunk commented 2 years ago

Alright, good to know. Actually have a bit of time to work on this today, so been transitioning the PR to use the new (at least compared to what omnifit master uses now) astropy package template instead of astropy_helpers.

My intent is to try and get the fixes implemented in such a way that I can automate most of the build/release checking on my end and not have to intervene much in package maintenance in the future, unless some dependency updates end up breaking the whole thing.

I can't promise that I'll do anything about e.g. the mentioned astropy ecosystem integrations in a reasonable timeframe, though. If I end up touching Omnifit to that degree, there's a number of other improvements I'd rather do first as by my current standards I'm not terribly happy with its fitting methodology in the first place. But that's a bigger issue to tackle.