Skip to content

MNT fix test following scipy.stats.mode change in scipy development version #25393

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 2 commits into from
Jan 18, 2023
Merged

MNT fix test following scipy.stats.mode change in scipy development version #25393

merged 2 commits into from
Jan 18, 2023

Conversation

lesteve
Copy link
Member

@lesteve lesteve commented Jan 13, 2023 •
edited by jjerphan
Loading

Partially address #25202.

One of the failure of scipy-dev see https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=50872&view=logs&j=dfe99b15-50db-5d7b-b1e9-4105c42527cf&t=ef785ae2-496b-5b02-9f0e-07a6c3ab3081

scipy.stats.mode has changed its returned array shape when axis=None and keepdims=True. See scipy/scipy#17561 for more details.

import numpy as np

import scipy.stats

arr = np.arange(24).reshape(6, 4)
weights = np.ones_like(arr)

mode, score = scipy.stats.mode(arr, axis=None, keepdims=True)

print(mode.ndim)

prints 2 in scipy development version and 1 in scipy latest release.

To be honest I am not sure about our exit strategy with this _mode thing ... because there will be a point where we still have to deal with the fact that sklearn.utils.extmath.weighted_mode is inconsistent with scipy.stats.mode for recent scipy versions.

Related to #23633

@lesteve lesteve changed the title MNT fix test following scipy dev change MNT fix test following scipy.stats.mode change in scipy development version Jan 13, 2023
jjerphan
jjerphan approved these changes Jan 17, 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 given that the CI passes.

@jjerphan jjerphan added the Quick Review For PRs that are quick to review label Jan 17, 2023
@lesteve
Copy link
Member Author

lesteve commented Jan 17, 2023 •
edited
Loading

Note: this PR alone is not enough to make scipy-dev green. For example, #25212 needs to be merged as well.

ogrisel
ogrisel approved these changes Jan 18, 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 as well. This is a temporary fix anyway. We will see later how to cleanly adapt the code base once we drop support for scipy > 1.11.

@ogrisel ogrisel merged commit d60af3d into scikit-learn:main Jan 18, 2023
@lesteve lesteve deleted the fix-scipy-mode-change branch January 18, 2023 12:58
@jjerphan jjerphan mentioned this pull request Jan 19, 2023
Merged
jjerphan added a commit to jjerphan/scikit-learn that referenced this pull request Jan 20, 2023
@lesteve @jjerphan
MNT fix test following scipy dev change (scikit-learn#25393)
432e71d 
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
jjerphan added a commit to jjerphan/scikit-learn that referenced this pull request Jan 20, 2023
@lesteve @jjerphan
MNT fix test following scipy dev change (scikit-learn#25393)
4dc532d 
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
jjerphan added a commit to jjerphan/scikit-learn that referenced this pull request Jan 23, 2023
@lesteve @jjerphan
MNT fix test following scipy dev change (scikit-learn#25393)
3905d41 
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
adrinjalali pushed a commit that referenced this pull request Jan 24, 2023
@lesteve @jjerphan @adrinjalali
MNT fix test following scipy dev change (#25393)
0542c11 
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
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
module:utils Quick Review For PRs that are quick to review
Projects
None yet
Milestone
No milestone
Development

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