-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
ENH: stats.chisquare/power_divergence: add array API support #20753
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
case.f_obs, case.f_exp, case.ddof, case.axis, | ||
2/3, case.cr) | ||
|
||
def test_basic_masked(self): |
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.
This didn't actually pass masked arrays into the function because check_power_divergence
converted mobs
back into a regular array, so I just deleted it.
def _m_sum(a, *, axis, preserve_mask, xp): | ||
if np.ma.isMaskedArray(a): | ||
sum = a.sum(axis) | ||
return sum if preserve_mask else np.asarray(sum) | ||
return xp.sum(a, axis=axis) | ||
|
||
|
||
def _m_mean(a, *, axis, keepdims, xp): | ||
if np.ma.isMaskedArray(a): | ||
return np.asarray(a.mean(axis=axis, keepdims=keepdims)) | ||
return xp.mean(a, axis=axis, keepdims=keepdims) |
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.
These and the _m_
functions above will be removed in two versions, when masked array support is removed.
|
||
ddof = np.asarray(ddof) | ||
expected_p = stats.distributions.chi2.sf(expected_stat, | ||
num_obs - 1 - ddof) | ||
assert_allclose(p, expected_p) | ||
|
||
def test_basic(self): |
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.
@parametrize
d now.
xp_assert_close(stat, xp.asarray([5.71428571, 2.66666667])) | ||
xp_assert_close(pval, xp.asarray([0.01682741, 0.10247043])) | ||
|
||
def test_power_divergence_against_cressie_read_data(self, xp): |
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.
Moved up from below.
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, changes are clear, same with the tests.
Thanks @tupui! |
Reference issue
Toward gh-20544
What does this implement/fix?
Adds array API support to
stats.chisquare
andstats.power_divergence
. Preserves but deprecates support for masked arrays per generalization of the conversation aboutgstd
.Additional information
Consider reviewing diff with "Hide whitespace" checked. I moved some test functions into classes.