-
Notifications
You must be signed in to change notification settings - Fork 21
Some nullable operations can throw exceptions #116
Comments
Actually, no operation is really safe for all possible types, even bitstypes. For example, a checked integer type would throw an error in case of overflow with Since you seem to be willing to work on Nullable, would you want to open a PR against Base to implement the operators which currently live in this package, but in fully safe approach? Cf. #111. |
Would a more appropriate name be |
I proposed Whatever its name, it would prevent the "overflow problem" by forcing a checked integer type to use the slow path in which the operation is only applied when the value is not missing. In the original example, |
I see now. Thank you for explaining. |
@nalimilan Could you point to any discussion where I am not a big fan of autolifting or autovectorization, but sane syntax is needed for elementwise lifted operations, and otherwise one would need to autolift elementwise operations on |
@TotalVerb IIRC there was no discussion -- I believe I included |
@davidagold Thanks for the explanation. These reducing operations have been reimplemented here anyway, so I think we are better off using explicit lifting, especially once |
AFAIK it is available on 0.5 (and on 0.4 using Compat), so we can start deprecating the lifting version as soon as your broadcast implementation is merged. That won't be possible immediately for |
@TotalVerb Also, I meant to ask, what do you mean by an "elementwise equivalent" --- i.e., this notion according to which you differentiate between operators such as |
There's a weird interaction with JuliaLang/julia#16961: in that PR, lifting on a single That's not necessarily an issue, but it means |
@nalimilan didn't you bring up this automatic lifting behavior of |
Not sure, there have been so many discussions in different places. The problem if we get rid of the automatic lifting with |
I'm pretty sure the reason I put that behavior in there in the first place was for performance --- if no entries are null, just treat it like broadcasting over a regular array. I think I didn't realize that I was conflating that optimization with lifting behavior. Is there anything wrong with |
Fixed by #119. |
Not all of the operations defined here are safe, because some operations are not defined on the entire domain, even if the types are bitstypes. Null values should not cause exceptions, especially in unpredictable ways.
Example:
(this happens to crash on my REPL fairly often, but your mileage may vary)
It's somewhat rare to actually have issues, but the following three operations should definitely be removed:
%
(doesn't work with 0)^
(doesn't work with negative exponents)sqrt
(doesn't work with negatives)The text was updated successfully, but these errors were encountered: