-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
BUG: incorrect equality of np.int64 and np.uint64 #8851
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
Comments
Is this really desirable?
Clearly though, in our case the value was not preserved |
Note that |
The compromise does seem particularly broken for comparison operators, since for those there is no need to upcast to get the correct answer. @eric-wieser - possibly here too one might want a separate |
Can we just do this everywhere? Obvious candidates:
I guess it's not clear whether I'd be in favor of disabling safe casting from |
Similar: #5746 |
I'd like to have a look at how to patch comparisons for |
Okay, it's going to be somewhat trickier than I thought. The code for creating ufunc objects in numpy is generated, and that logic is quite complex. You can start here: https://github.com/numpy/numpy/blob/master/numpy/core/code_generators/generate_umath.py#L407-L442 You will see that all of the comparisons have a simple The functions that you actually implement are here: https://github.com/numpy/numpy/blob/master/numpy/core/src/umath/loops.c.src#L916-L932 As you can see, they are also generated through an ad-hoc templating system. I'm not sure if you'll be able to make much use of the templating system; I expect the code will look different for each of the cases. In any case, make sure the functions you add are also declared in The Or you can add a new branch in this bit of code here to deal with whatever new scheme you come up with: https://github.com/numpy/numpy/blob/master/numpy/core/code_generators/generate_umath.py#L919-L964 |
@gfyoung: Take a look at #8853 for an example of adding these loops. Everything @rkern said above should apply too. It might be worth adding some special handling to make Of particular note - you need to handle both |
Is anyone still working on this? >>> np.can_cast(np.int64, np.float64, casting="safe")
True |
@umanwizard , I filed #13733 for that separately. |
@dcolascione FWIW it's discussed in this present thread, in Eric's comment |
Ah, okay. Thanks! |
Closing in favor of gh-12525, but stealing the clearer title :). |
This doesn't look at all desirable to me:
Frankly, this is PHP-like behaviour, and not something I want to see happening quietly in python
Clearly what's happening here is that
np.equal
is upcasting both arguments to float, since combining auint64
andint64
requires aint128
for storage.I don't think relational operators should be allowed to make this conversion, and should either error, or do more work to determine the correct result.
Perhaps in general, we should warn when a
(uint64,int64)->float
conversion occurs, and ask that it be written explicitly? At the very least, I don't think this operation is appropriately covered bycasting="safe"
The text was updated successfully, but these errors were encountered: