MAINT: fix skellam distribution tests #22971
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dschmitz89
commented
May 12, 2025
[skip ci]
mdhaber
approved these changes
May 12, 2025
There was a problem hiding this comment.
Thanks @dschmitz89! The test is skipped as xslow in line 1116, so it's not running in CI. Please remove 'skellam' from the list and this will LGTM, but I hope @steppi or someone can take a quick look at the compiled code.
|
Oops! So it was indeed very slow. Let's see if tests pass otherwise, then we can add |
mdhaber
reviewed
May 12, 2025
scipy/stats/tests/test_continuous.py
Outdated
| @@ -1113,7 +1113,7 @@ def test_rv_generic(self, i, distdata): | |||
| 'johnsonsb', 'kappa4', 'ksone', 'kstwo', 'kstwobign', 'norminvgauss', | |||
| 'powerlognorm', 'powernorm', 'recipinvgauss', 'studentized_range', | |||
| 'vonmises_line', # continuous | |||
| 'betanbinom', 'zipf', 'logser', 'skellam'} # discrete | |||
| 'betanbinom', 'zipf', 'logser'} # discrete | |||
There was a problem hiding this comment.
Suggested change
| 'betanbinom', 'zipf', 'logser'} # discrete | |
| 'betanbinom', 'zipf', 'logser', 'skellam'} # discrete |
|
Great; everything passed except for the slow test. The suggestion #22971 (review) can be committed with |
[skip ci]
This reverts commit eb5527e. [skip ci]
j-bowhay
approved these changes
May 13, 2025
johnmdusel
pushed a commit
to johnmdusel/scipy
that referenced
this pull request
May 13, 2025
* TST: add test case extreme input for ncx2 pdf * TST: remove skellam from makedistribution skip list * MAINT: catch errors in ncx2 pdf * TST: Remove skellam from slow list --------- Co-authored-by: Matt Haberland <mhaberla@calpoly.edu>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference issue
Closes #22956
What does this implement/fix?
This should help with the recent discrete distribution updates. I do not understand the context fully though. Locally, I still see failures for other distributions.
Long term, it would be better to have a unified wrapping mechanism for boost functions that handle C++ exceptions generally. Let's fight that battle another day though.