grizli
grizli copied to clipboard
gbrammer
Updating to Polynomial
As of SciPy 1.12.0 polyfit and polyval can no longer be directly imported from SciPy. This caused import errors when running grizli. I suspect this is due to the fact that polyfit and polyval are part of the old polynomial API, which has been upgraded with NumPy Polynomial package, primarily resulting in greater numerical stability. While I could have just fixed the imports, I went through and updated the code to use the new Polynomial API. If preferred, I can simply replace the scipy imports with the correct/not broken numpy imports.
I like the modernization idea, but the wholesale changes to the new Polynomial behavior are a bit risky, particularly in the way the order of the polynomial coefficients are interpreted is exactly reversed! Those coeffs[::-1] throughout aren't pretty, but at least everything (sort of) works!
Could we just switch to from numpy.polynomial.polynomial import polyfit, polyval for now and file switching to the Polynomial objects as a WIP Issue?
Sounds good, I've submitted PR #207 to address the short-term issues, but will continue testing against this PR and keep this updated.
This PR now passes all CI checks and has been tested on real data to ensure it gives the same results.
Thanks for keeping an eye on this. Can you update minimum versions on the numpy and scipy dependencies in setup.cfg to ensure compatibility with these changes (e.g., cumtrapz > cumulative_trapezoid)? I added scipy<1.14 the other day to fix a bug in the external (and apparently stale) peakutils module, which uses scipy.integrate.simps that has been renamed to simpson.
This is a good point, we actually maintain greater compatibility with this change. The "new" Polynomial syntax has been around in NumPy since 1.4, which is over 10 years old at this point. It's true that scipy has finally removed the deprecated "simps/cumtrapz" syntax, I've forked peakutils and made the necessary updates, I wonder if we can pull from there for now. I wonder if there is a more modern package with the same functionality.
I've just pushed the change requiring NumPy >=1.4. There are likely other parts of grizli that would require newer NumPy versions, but this is a start.