MAINT: sparse: Simplify sputils.isintlike #7864
Conversation
This works around scipygh-7860 by not calling np.isscalar, as suggested by @eric-wieser. Some objects that fail sputils.isscalarlike will pass the np.ndim check (i.e. object), but these are handled by the try-except check, which should be cheap for anything where np.ndim(x) == 0.
|
Should this actually use |
|
Yes, but that would be a compatibility break. Perhaps Of course, really the error should propagate too |
|
yeah, for 1.0.0 we probably want the backward compatible solution, we can iterate more in 1.1.0 |
rgommers
added
maintenance
|
I added the suggested deprecation warning, for the case where |
scipy/sparse/sputils.py
Outdated
| return False | ||
| if loose_int: | ||
| warnings.warn("Inexact indices to sparse matrices are deprecated " | ||
| "as of scipy 1.0") |
There was a problem hiding this comment.
Ought to be a DeprecationWarning
[ci skip] Test case needs warning filter.
|
@eric-wieser I thought I was forgetting something with that warning... I also added a filter to the unit test so pytest doesn't fail when the warning is thrown. |
| @@ -189,12 +190,20 @@ def isintlike(x): | |||
| """Is x appropriate as an index into a sparse matrix? Returns True | |||
| if it can be cast safely to a machine int. | |||
| """ | |||
| if not isscalarlike(x): | |||
There was a problem hiding this comment.
Probably worth commenting that this is strictly an optimization - this is already covered implicitly by the other branches
There was a problem hiding this comment.
I added a comment to that effect.
scipy/sparse/sputils.py
Outdated
| @@ -189,12 +190,20 @@ def isintlike(x): | |||
| """Is x appropriate as an index into a sparse matrix? Returns True | |||
| if it can be cast safely to a machine int. | |||
| """ | |||
| if not isscalarlike(x): | |||
| if np.ndim(x) != 0: | |||
There was a problem hiding this comment.
Why is this before the call to operator.index?
EDIT: ok, I see now.
| return False | ||
| if loose_int: | ||
| warnings.warn("Inexact indices into sparse matrices are deprecated", | ||
| DeprecationWarning) |
There was a problem hiding this comment.
Shouldn't the DeprecationWarning be raised unconditionally if operator.index fails?
There was a problem hiding this comment.
No, the warning should only appear for places where isintlike(x) has the "wrong" value. If the old conversion gives the same result as operator.index, there's nothing to warn about - the False already gets turned into an error down the line,
[ci skip]
This works around gh-7860 by not calling np.isscalar, as suggested by @eric-wieser here.
Some objects that fail
sputils.isscalarlikewill pass thenp.ndimcheck (i.e.object), but these are handled by the try-except check, which should be cheap for anything wherenp.ndim(x) == 0.