MAINT: special: factorial clean-ups #19989
Conversation
This prepares for an upcoming change; without this helper, supporting complex inputs becomes essentially unreadable.
avoid overloading already-complicated test_factorialk_scalar_corner_cases even further (and prepares for upcoming changes that expand it)
No functional change; reduce diff for upcoming work
having 1.1 cast to integer doesn't extend the test coverage in any way
…k} tests default for rtol is only 1e-7, see https://numpy.org/doc/stable/reference/generated/numpy.testing.assert_allclose.html
github-actions
bot
added
scipy.special
maintenance
|
The failures here are just minor tolerance violations (because I tightened things from the very loose default of |
| @@ -2088,55 +2089,79 @@ def test_expm1more(self): | |||
| assert_array_almost_equal(ex1,exrl1,8) | |||
|
|
|||
|
|
|||
| def assert_really_equal(x, y, rtol=None): | |||
There was a problem hiding this comment.
drive-by comment as I don't have time to review in detail now, but this seems to be repeating some work that was done in gh-19186 - perhaps you could use scipy._lib._array_api.xp_assert_equal(..., check_namespace=False)? (see the comment in _check_scalar from that PR for the discussion of how we're testing 0-d arrays vs scalars in cluster right now)
There was a problem hiding this comment.
Are we will willing to use xp_assert_* also for regular functions that don't use anything about the array API?
Also, would people be willing to enhance the strict check to distinguish nans vs. complex nans? I'm not introducing this in this PR, but I need something along the lines of ed245ec (see #19988). If I should incorporate that into xp_assert_{equal,close}, that's OK for me as well.
There was a problem hiding this comment.
would people be willing to enhance the strict check to distinguish nans vs. complex nans?
cc @mdhaber
Are we will willing to use xp_assert_* also for regular functions that don't use anything about the array API?
From my perspective it makes sense since ultimately they will be replaced with xp-assertions anyway. The only superfluous check for now can be turned off with check_namespace=False.
There was a problem hiding this comment.
enhance the strict check to distinguish nans vs. complex nans
Do you mean:
-
distinguishing between
-
a complex number
xfor whichnp.isnan(x)is Trueand
-
a complex number
xfor whichnp.isnan(x.real) and np.isnan(x.imag)is True
or
-
-
distinguishing between
-
a number
xfor whichnp.isnan(x)is Trueand
-
a number
xfor whichnp.isnan(x) and np.issubdtype(x.dtype, np.complexfloating)is True
-
? (One of the points of the strict checks is to check dtype, so I think the second thing is covered already.)
There was a problem hiding this comment.
The type equality is already included in the strict mode, but I want to distinguish NaN from NaN + 0j from NaN + NaN*j, so this:
a complex number
xfor whichnp.isnan(x.real) and np.isnan(x.imag)is True
|
Just saw numpy/numpy#24680 & numpy/numpy#24770 in the numpy 2.0 release notes. That's even nicer (and IMO enough if we're testing this on numpy 2.0 only). Thanks a lot @mdhaber! 🙏 |
lucascolley
changed the title
Preparation for complex support in factorial functions (see #18409), which needs some preparations in the tests, as well as an auxiliary function
_is_subdtypeto be able to sanely express the necessary subtype conditions.No functional changes in this PR, aside from changing the text of some warning messages. Also tightens some tests that were using the woefully inadequate default of
rtol=1e-7inassert_allclosePreview of what this PR is preparing for can be found in #19988