Skip to content
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

MAINT: stats._contains_nan: fix bug when -inf and inf are in sample #20467

Merged
merged 3 commits into from May 14, 2024

Conversation

mdhaber
Copy link
Contributor

@mdhaber mdhaber commented Apr 14, 2024

Reference issue

Closes gh-20386

What does this implement/fix?

gh-20386 reported that stats.kstest returned NaNs when both positive and negative infinities were present in the sample. This fixes the (very old) underlying bug.

Additional information

This includes all the gh-20292 commits, so that should merge first.

I could add a test for _contains_nan directly if desired. (Currently, this just adds a test for the reported issue in kstest.)

@mdhaber mdhaber added scipy.stats maintenance Items related to regular maintenance tasks labels Apr 14, 2024
@github-actions github-actions bot added scipy._lib Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org CI Items related to the CI tools such as CircleCI, GitHub Actions or Azure DX Everything related to making the experience of working on SciPy more pleasant labels Apr 14, 2024
@lucascolley lucascolley added this to the 1.14.0 milestone Apr 14, 2024
@mdhaber mdhaber removed Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org CI Items related to the CI tools such as CircleCI, GitHub Actions or Azure DX Everything related to making the experience of working on SciPy more pleasant labels Apr 14, 2024
@mdhaber mdhaber marked this pull request as ready for review April 21, 2024 16:26
@mdhaber mdhaber requested a review from j-bowhay May 6, 2024 01:03
Copy link
Member

@j-bowhay j-bowhay left a comment

Choose a reason for hiding this comment

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

Thanks @mdhaber

@j-bowhay j-bowhay merged commit 8d91da2 into scipy:main May 14, 2024
30 checks passed
contains_nan = xp.isnan(xp.sum(a))
else:
contains_nan = xp.any(xp.isnan(a))
if a.size == 0:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I should have used _array_api.size here. Will do in gh-20667.

@tylerjereddy tylerjereddy added the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label May 14, 2024
@tylerjereddy
Copy link
Contributor

Title is prefixed with MAINT: but says "fix bug," so I've tentatively added a backport label

@mdhaber mdhaber added backport-candidate This fix should be ported by a maintainer to previous SciPy versions. and removed backport-candidate This fix should be ported by a maintainer to previous SciPy versions. labels May 15, 2024
@mdhaber
Copy link
Contributor Author

mdhaber commented May 15, 2024

Might be tricky to backport because this is mixed with adding array API support to the function, which is a new feature. Up to you, though.

@tylerjereddy tylerjereddy removed the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Items related to regular maintenance tasks scipy._lib scipy.stats
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: scipy.stats.kstest returns NaN starting in scipy 1.12
4 participants