Skip to content

CI Fix scipy-dev issues related to numpy 2.0 changes #27190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Aug 29, 2023
Merged

CI Fix scipy-dev issues related to numpy 2.0 changes #27190

merged 5 commits into from
Aug 29, 2023

Conversation

lesteve
Copy link
Member

@lesteve lesteve commented Aug 28, 2023 •
edited
Loading

numpy.core.numeric.ComplexWarning was removed in numpy dev recently
https://github.com/numpy/numpy/pull/24376/files#diff-68601ddf5a8d7364167feb9c1546348682ed4adbd37ab7c24aa66a43fb874da5

This is causing the scipy-dev build to fail early see this build for example with the following stack-trace:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/vsts/work/1/s/sklearn/__init__.py", line 83, in <module>
    from .base import clone
  File "/home/vsts/work/1/s/sklearn/base.py", line 19, in <module>
    from .utils import _IS_32BIT
  File "/home/vsts/work/1/s/sklearn/utils/__init__.py", line 22, in <module>
    from ._param_validation import Integral, Interval, validate_params
  File "/home/vsts/work/1/s/sklearn/utils/_param_validation.py", line 15, in <module>
    from .validation import _is_arraylike_not_scalar
  File "/home/vsts/work/1/s/sklearn/utils/validation.py", line 25, in <module>
    from numpy.core.numeric import ComplexWarning  # type: ignore
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ImportError: cannot import name 'ComplexWarning' from 'numpy.core.numeric' (/usr/share/miniconda/envs/testvenv/lib/python3.11/site-packages/numpy/core/numeric.py)

Edit: more fixes for numpy 2.0 changes while I was at it:

  • np.infty -> np.inf
  • np.NaN -> np.nan
  • np.float_ -> np.float64

@github-actions
Copy link

github-actions bot commented Aug 28, 2023 •
edited
Loading

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 65fd21c. Link to the linter CI: here

ogrisel
ogrisel approved these changes Aug 28, 2023
View reviewed changes
Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @lesteve!

@lesteve lesteve changed the title MNT Add numpy ComplexWarning to sklearn.utils.fixes for numpy 2 support CI Fix scipy-dev issues related to numpy 2.0 changes Aug 28, 2023
@lesteve
Copy link
Member Author

lesteve commented Aug 29, 2023 •
edited
Loading

There is one remaining error (see build log) that seems to be a change in scipy.sparse.dok_array. Maybe @jjerphan has some insights into this?

The following snippet fails with numpy and scipy dev but works fine with the latest released version of scipy (1.11.2) and numpy (1.25.2):

import numpy as np

from scipy.sparse import dok_array

array = np.ones((2, 3))
sparse_array = dok_array(array)

np.may_share_memory(array, sparse_array)

Traceback:

---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
File /tmp/test-sparse.py:8
      5 array = np.ones((2, 3))
      6 sparse_array = dok_array(array)
----> 8 np.may_share_memory(array, sparse_array)

File ~/micromamba/envs/scikit-learn-dev-deps/lib/python3.11/site-packages/scipy/sparse/_base.py:251, in _spbase.__iter__(self)
    249 def __iter__(self):
    250     for r in range(self.shape[0]):
--> 251         yield self[r, :]

File ~/micromamba/envs/scikit-learn-dev-deps/lib/python3.11/site-packages/scipy/sparse/_index.py:53, in IndexMixin.__getitem__(self, key)
     51     return self._get_intXint(row, col)
     52 elif isinstance(col, slice):
---> 53     self._raise_on_1d_array_slice()
     54     return self._get_intXslice(row, col)
     55 elif col.ndim == 1:

File ~/micromamba/envs/scikit-learn-dev-deps/lib/python3.11/site-packages/scipy/sparse/_index.py:40, in IndexMixin._raise_on_1d_array_slice(self)
     37 from scipy.sparse import sparray
     39 if isinstance(self, sparray):
---> 40     raise NotImplementedError(
     41         'We have not yet implemented 1D sparse slices; '
     42         'please index using explicit indices, e.g. `x[:, [0]]`'
     43     )

NotImplementedError: We have not yet implemented 1D sparse slices; please index using explicit indices, e.g. `x[:, [0]]`

We did not see it on August 18 according to #27042 (comment) so this is quite a recent change.

@jjerphan
Copy link
Member

I think scipy/scipy#18929 is the culprit.

@lesteve
Copy link
Member Author

lesteve commented Aug 29, 2023

Yep this is what git bisect is telling me as well

@jjerphan jjerphan mentioned this pull request Aug 29, 2023
Merged
jjerphan
jjerphan approved these changes Aug 29, 2023
View reviewed changes
Copy link
Member

@jjerphan jjerphan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Thank you, @lesteve.

@jjerphan
Copy link
Member

Should we merge despite the regression in SciPy?

@lesteve
Copy link
Member Author

lesteve commented Aug 29, 2023

Should we merge despite the regression in SciPy?

I think so, this is a clear improvement to the current status (early fail because of ComplexWarning issue) and this gets us closer to having a green scipy-dev build (which is quite a moving target these days ...).

@jjerphan jjerphan merged commit 1821c3c into scikit-learn:main Aug 29, 2023
@lesteve lesteve deleted the complex-warning branch August 29, 2023 11:18
@betatim betatim mentioned this pull request Aug 29, 2023
Closed
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Aug 29, 2023
@lesteve @glemaitre
CI Fix scipy-dev issues related to numpy 2.0 changes (scikit-learn#27190
132f882 
)
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Sep 18, 2023
@lesteve @glemaitre
CI Fix scipy-dev issues related to numpy 2.0 changes (scikit-learn#27190
daa93cf 
)
jeremiedbb pushed a commit that referenced this pull request Sep 20, 2023
@lesteve @jeremiedbb
CI Fix scipy-dev issues related to numpy 2.0 changes (#27190)
8d40fc1
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
@lesteve @REDVM
CI Fix scipy-dev issues related to numpy 2.0 changes (scikit-learn#27190
45dface 
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogrisel ogrisel ogrisel approved these changes

@jjerphan jjerphan jjerphan approved these changes

Assignees
No one assigned
Labels
Build / CI module:utils No Changelog Needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lesteve @jjerphan @ogrisel