Skip to content
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

BUG: timedelta and float promotion sometimes works #16566

Closed
seberg opened this issue Jun 10, 2020 · 4 comments · Fixed by #16592
Closed

BUG: timedelta and float promotion sometimes works #16566

seberg opened this issue Jun 10, 2020 · 4 comments · Fixed by #16592

Comments

@seberg
Copy link
Member

seberg commented Jun 10, 2020

The following examples do not lead to symmetric results:

np.promote_types("float32", "m8")
# returns "m8", which is probably wrong
np.promote_types("m8", "float32")
# raises TypeError.

The same is true for all other floating point types. I wonder if there is some strange logic to this, as a hack to allow certain ufunc calls, but I did not find one yet.

@anirudh2290
Copy link
Member

Looks like at least np.multiply supports this.

import numpy as np
print(np.multiply(np.timedelta64(1), 1.2))

I think the above should also be considered a bug.

@anirudh2290
Copy link
Member

I think the question is it always true that for ufuncs "casting rules are implemented by the question of when a data type can be cast “safely” to another data type" as described in the documentation. Looks like there are exceptions like above where casting isnt safe but we have made the ufunc to work. I guess in this case the promote_types should still say invalid type promotion (for both np.promote_types("float32", "m8") and np.promote_types("m8", "float32")). Whether we need to continue to allow ufunc to work can be decided on a case by case basis ?

@seberg
Copy link
Member Author

seberg commented Jun 12, 2020

Yes, we have to clearly distinguish that np.promote_types must be a generic rule which is always reasonable. Which is why I like the term "common dtype". This is e.g. the dtype that can be used with np.concatenate.

Each ufunc individually can decide to implement a specific operation using "common dtype" or other logic. We specifically do it for multiple and timedelta64, so that is a separate issue indeed.

I suppose we do not actually use this logic in many places, since ufuncs mostly rely on np.can_cast(..., ...) as you note. And while that is typically aligned with type promotion, it is not here.

@anirudh2290
Copy link
Member

Given the above discussion I went ahead and added the fix for promote_types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants