Repository navigation
DEP: Deprecate promotion of numbers and bool to string - #18116
Conversation
seberg
added
the
56 - Needs Release Note.
031f935 to
861a0fa
Compare
seberg
removed
the
56 - Needs Release Note.
861a0fa to
94e9a27
Compare
|
There is a warning in doc building: |
94e9a27 to
25e5572
Compare
There was a problem hiding this comment.
| Comparisons and universal functions are not affected by this. | |
| Comparisons, universal functions, and casting are not affected by this. |
25e5572 to
f6a813e
Compare
|
I restarted Azure since it hit gh-18119 on the 64-bit run (failed to find nm.exe) |
There was a problem hiding this comment.
Something I randomly realized: I could change this so that it it actually raises the FutureWarning here. That is: We could differentiate between "no promotion found/possible" and an error while trying to promote).
But, I am not sure it makes things better here, and another solution for that distinction is to create a PromotionError type (assuming that invalid promotions are unlikely to be ever speed relevant).
There was a problem hiding this comment.
It seems we would want to make sure a warning is noticed when promotion fails. Should we merge as-is and enhance in another PR or add it here?
There was a problem hiding this comment.
Sorry, just to be clear (just to be sure), this affects:
warnings.simplefilter("error", FutureWarning)
np.array([0, "a"])
which would "opt in" to the new behaviour currently. Happy to change it. But maybe the distinction for custom errors doesn't matter too much. If we introduce a PromotionTypeError to allow custom errors (if that even makes sense for an operator), we could still catch PromotionTypeError explicitly.
There was a problem hiding this comment.
I forgot that this also affects, the second step, which would be the one that finds the correct unit for two different datetimes. If you think its worthwhile for this warning, I can look into it. Otherwise, I think swapping out to an explicit error type might be more elegant at some point (if we ever care). Since we should also need to say that __common_instance__ should return NotImplemented for such impossible cases, which seems not really that great to me.
|
@seberg needs rebase. |
f6a813e to
5164312
Compare
5164312 to
38bda3c
Compare
|
Where do we stand with this? Has it been discussed on the mailing list? Does it need to be or is this niche enough we can merge it as is? |
|
This has been mentioned multiple times on the list, just not super recently. |
|
Thanks @seberg |
This deprecates promotion of strings and numbers (and bool). This an absolutely necessary step to make things sane at some point... (marking as draft for now, as it is based off the fix for promotion with half precision and string).
Some notes:
safecasting to string is also weird, but most places where we use "safe casting" as logic (i.e. ufuncs) should be using promotion instead, so I just don't care much anymore how we define "safe" casting. (Safe casting is just another casting level, and not a fundamental operator used for any logic.)np.arrayuses atry/except.