-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
FIX/MNT Compensated for changes in stats.mode
to keep internal behavior
#23640
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.
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
Conversation
It seems that your fix does not work as intended (for released versions of scipy). BTW, could you please report the problem upstream to notify scipy developers of the breaking change? It's probably not intended and maybe we shouldn't have to change anything in the scikit-learn code base. |
For reference, here is the failure we observed with scipy-dev: def test_uniform_weights():
# with uniform weights, results should be identical to stats.mode
rng = np.random.RandomState(0)
x = rng.randint(10, size=(10, 5))
weights = np.ones(x.shape)
for axis in (None, 0, 1):
mode, score = stats.mode(x, axis)
mode2, score2 = weighted_mode(x, weights, axis=axis)
> assert_array_equal(mode, mode2)
E AssertionError:
E Arrays are not equal
E
E (shapes (5,), (1, 5) mismatch)
E x: array([3, 3, 7, 2, 1])
E y: array([[3., 3., 7., 2., 1.]]) while this used to pass with previous versions of scipy. |
I reported the change of shape in the return value of |
Adapted test to account for scipy-version along w/ a clarifying comment. I could have done it implicitly, e.g. by checking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually let's not merge this if we want to follow the route of #23633 (comment).
Reference Issues/PRs
Related to #23626
Related to #23633
What does this implement/fix? Explain your changes.
Expands the dimensions of the outputs to
stats.mode
to keep test consistent with current internal behavior ofweighted_mode
.Any other comments?