flav-io / flavio

A Python package for flavour physics phenomenology in the Standard model and beyond
http://flav-io.github.io/
MIT License
71 stars 62 forks source link

New scipy version (1.5.0 and above) causes import error #110

Closed MJKirk closed 4 years ago

MJKirk commented 4 years ago

In https://github.com/scipy/scipy/commit/d1c119150d1c11eb9025b21afc5505ff3ee42911 they renamed scipy/integrate/quadrature.py → scipy/integrate/_quadrature.py so now https://github.com/flav-io/flavio/blob/7e73035897be05bffd5bcf5af80108ed890753cd/flavio/math/integrate.py#L4 gives an error when importing flavio It's a simple fix: change that line to

from scipy.integrate import AccuracyWarning

Happy to submit a pull request if you want.

peterstangl commented 4 years ago

Thank you for reporting the issue and for offering to submit a PR!

For the solution, we just should make sure that it also still works with older scipy versions. As far as I see, your solution should work with older versions but it might be good to explicitly check this.

MJKirk commented 4 years ago

I just checked and unfortunately it does not work, because AccuracyWarning wasn't in __all__ until that change, and so it didn't get pulled into scipy.integrate. I don't think there's anything that would work for both, so we either have to check against the scipy version and have two different import statements, or require scipy >= 1.5.0 for flavio and just have the newer form.

peterstangl commented 4 years ago

Instead of checking for the scipy version, we could use try and except:

try:
    from scipy.integrate import AccuracyWarning
except Exception:
    from scipy.integrate.quadrature import AccuracyWarning

This would first try to use the new implementation and would fall back to the old one in case of an exception and there would be no need to know the scipy version. Of course this is not the nicest solution and in the future we might want to require scipy >= 1.5.0. But for now I think it shouldn't be necessary to break backward compatibility just because of this small change in scipy.

DavidMStraub commented 4 years ago

What about

warnings.filterwarnings("ignore", module="scipy.integrate")

? Haven't tried it, but would be more concise.

peterstangl commented 4 years ago

Fixed by PR #111.