BUG: Fix problems with obj2sctype #9522
Conversation
dtype(t) already does this in descriptor.c This means that `obj2sctype(long)` now returns `np.int64` rather than `None`, because when you duplicate code, its easy to forget something in the second place.
|
Needs compatibility note in the release notes. |
|
Done |
b62ba3e to
917700c
Compare
| else: | ||
| # if the type changed, we can safely overwrite absx | ||
| abs(absx, out=absx) | ||
| absx = abs(x) |
There was a problem hiding this comment.
This is not quite the same as asfarray casts to float64 type but abs does not. In particular this will affect float16 type. OTOH, float16 is preserved by the other norms. Hmm... IIRC, this function has had various type problems over the years. So the change may be justified, but might be worth something in the release notes.
There was a problem hiding this comment.
The comment stated this was done for integer types. Without conversion, those now would seem to have a bigger risk of overflow in the .reduce below. I guess converting to float is not unreasonable since the **(1/ord) at the end will do that anyway. On the other hand, I prefer the simplicity of this change, and it is also what is done above for ord=2 (surely, the most common case).
There was a problem hiding this comment.
Integer types are already gone by the time we get here.
There was a problem hiding this comment.
Ah, you're right, there is an asfloat above (at least for integer arrays - but special-casing object definitely feels bad). So, the comment was out of date, then, and there would seem to be no reason not to go with your simplification.
There was a problem hiding this comment.
Yep, as mentioned in the commit, #7088 made this essentially dead code, but didn't notice.
There was a problem hiding this comment.
@charris: Good catch on the behaviour change - release notes updated
There was a problem hiding this comment.
The old behaviour was particularly weird because norm(complex64) -> float32 but norm(float32) -> float64
As of numpygh-7088, x is always a float array anyway
ef3c41b to
351fe97
Compare
|
Thanks Eric. |
Previously
obj2sctype(long) is None, which conflicts withnp.dtype(long).type is np.int64If these use the same code path, then this won't happen.
Also,
obj2sctypeis not suitable for use in any public API, as it silently allows arrays to be passed in place of dtypes.