ricemunk / omnifit Goto Github PK

One common thing that is usually done with ice fitting results is being able to calculate the column density of the ice represented by the fit.

A few different ways of doing this currently exist:

• Integrate over a feature, multiply by an experimentally determined factor
• Take a peak value within a certain range, multiply by an experimentally determined factor
• Compare to unfitted spectrum with known quantity

Also stuff like CDE correction changes how these work, and has to be taken into account to get accurate results.

It would be very useful for Omnifit to be able to do this, instead of forcing the user to mess around with the exported fit results on their own.

Currently you can do something like spectrum1.y -= spectrum2.y to subtract one spectrum from another (assuming both have the same x), but this neither checks that the spectra are compatible, and it's also a bit clumsy.

It would be much nicer to be able to simply do spectrum1 -= spectrum2 to do the same thing, while also checking that it makes sense.

doing spec1.interpolate(spec2,clone=True) throws a warning about no units given, and also breaks the units in the new spectrum.

Might be related to cloning or something with interpolate.

Currently Omnifit can export its fit results as a pair of CSV and XML files. While this way of doing things works, it's not necessarily very user-friendly.

A few alternative formats to export stuff in would be:

• a FITS file
• a HDF file

Either of these would probably be superior to the home-cooked format Omnifit currently uses, and thus it would make sense to include support for them.

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/Scope
No further comments
Integration with Astropy ecosystem
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.
Documentation
The documentation is quite short at the moment and could do with a better narrative and more examples.
Testing
There are a number of tests but the continuous integration (Travis) doesn't appear to be running correctly.
Development status
No further comments
Python 3 compatibility
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.

Seems like one of the unit tests has started failing at some point. The following bit specifically seems to be raising an exception:
testfitter.add_analytical('gaussian',testpars,funcname='test gaussian') testfitter.perform_fit()
Fails with "fitting failed".

My guess: something has changed with lmfit which makes it not behave with omnifit. Should look into this when have time.

Steps to reproduce:

1. Create spectrum
2. Convolve spectrum using convolve

Expected result:
y of convolved spectrum should be of type astropy.units.Quantity

Actual result:
y of convolved spectrum is of type numpy.ndarray

This is an automated issue from Astropy. You might be using code removed in astropy v6.0.dev from astropy/astropy#14670 in the following files:

* omnifit/conftest.py

Please use https://docs.pytest.org/en/stable/how-to/capture-warnings.html instead.

If this issue is opened in error or irrelevant, feel free to close. We apologize for any inconvenience caused.

When tox attempts to run python -m pip install setuptools setuptools_scm wheel extension-helpers oldest-supported-numpy cython==0.29.14 it creates a whole slew of errors which seems to revolve around ImportError: DLL load failed while importing _ctypes: The specified module could not be found

Currently the CDEspectrum class makes use of complex refractive index values which are commonly calculated using the Kramers-Kronig (KK) relation equation. It is not a terribly complex calculation, yet it's not included as part of Omnifit.

An obvious improvement to Omnifit is to make it be included as part of the library, so CDEspectrum can be initialised without outside help.

The https://ricemunk.github.io/omnifit/ home page is returning a 404

While I see a .travis.yml file, it seems that it has never run for this repository. While I acknowledge that you probably run the tests before releases, having a CI system that is integrated with GitHub can be extremely useful.

It seems like a lot of the omnifit functionality requires lmfit. Should it be a requirement for installation?

Hello from Astropy Project!

The following updates to your package might be necessary to ensure compatibility with the latest stable version of Astropy:

• MPLBACKEND is now set to Agg in ci-helpers, packages expecting interactive plotting should override it in .travis.yml
• Astropy 3.1 is not compatible with Numpy <1.13 versions. If you want to keep testing these older Numpy versions, please use ASTROPY_VERSION=3.0 or ASTROPY_VERSION=LTS in your Travis CI matrix.
• Add sphinx-astropy as a package dependency if you are using astropy-helpers 3.1 or later. Otherwise, your documentation build (e.g., on ReadTheDocs) might fail.
• If you are using six that is bundled with Astropy, please consider using the standalone six package instead.

If these are no longer applicable to you, please close the issue.

This is an automated issue for packages that opted in for automated astropy-helpers update. If this is opened in error, please let @pllim know.

xref astropy/astropy-tools#108

Python 3 is not yet supported.

Python 3 should be supported.

Running the example full fit (see example in documentation) seemingly does the fit successfully, but never modifies fit parameters from their original guesses. Figure of expected fit result as shown in example is not reproduced.

This seems to happen on fresh installs of Omnifit using the dependencies installed through PyPi, so it may be related to some change which has happened in newer version of the dependencies; probably in ones involving minimization.

As recommended by the astropy folks and since it would be nice to reduce the number of dependencies, it might be a good idea to use astropy.modeling instead of lmfit as the fitting engine for omnifit.

To make me not forget to check whether or not this suits what omnifit needs to do, I'm creating this issue here to keep reminding me.