search

online-ml / river

🌊 Online machine learning in Python
https://riverml.xyz
BSD 3-Clause "New" or "Revised" License
5.03k stars 540 forks source link

Fixed `AttributeError` in metrics/roc_auc.py. #1568

Closed RavSS closed 2 months ago

RavSS commented 3 months ago

SciPy deprecated scipy.integrate.trapz long ago and it had instead been renamed to scipy.integrate.trapezoid. The ROCAUC() class uses the former instead of the latter (causing an AttributeError), with the latter being included from the start of SciPy version 1.6.0 (start of 2021). This is a small fix: I've added a simple hasattr() check for compatibility with versions that still have scipy.integrate.trapz and to use it if it's there over scipy.integrate.trapezoid, but it might be unnecessary, since those versions are outdated.

https://docs.scipy.org/doc/scipy/release/1.6.0-notes.html#scipy-integrate-improvements

gbolmier commented 2 months ago

Hey @RavSS, my apologies for the slow reply and thank you for the contribution!

I think it would make more sense to only use the new trapezoid function name instead and floor the SciPy dependency to 1.6.0. WDYT?

RavSS commented 2 months ago

I agree. It would make maintenance easier too; however, if the latest version of River still works with SciPy versions before 1.6.0 in other parts, then I think it's nice to keep the backwards compatibility at almost no cost for now.

e10e3 commented 2 months ago

I would like to point out that the current version of River already requires SciPy to be in version 1.12 or greater (see the pyproject.toml).

No harm would be done in dropping support for SciPy anterior to 1.6.

gbolmier commented 2 months ago

Edit: Too fast for me @e10e3!

@RavSS I just noticed that our SciPy version is already floored to 1.12.1, no need to support scipy<1.6.0 versions then:

https://github.com/online-ml/river/blob/51bb8349f6c79c346ef542a5d62c213e0067bcde/pyproject.toml#L33

Also Python >=3.10 were not supported before 1.6.