riptable icon indicating copy to clipboard operation
riptable copied to clipboard

In-place division uses floordiv instead of a true division

Open MarcMassar opened this issue 5 years ago • 2 comments

For FastArray of dtype=int, in-place division does floor div instead of true division, which is inconsistent with with the behavior of regular division. I think either riptable should throw when trying to do an in-place division (this is what numpy does), or should do a true division -- I prefer the later.

>>> rt.__version__
'1.0.25'
>>> a = rt.FA([1])
>>> b = rt.FA([2])
>>> a / b
FastArray([0.5])
>>> a /= b
>>> a
FastArray([0])

MarcMassar avatar Dec 22 '20 15:12 MarcMassar

The latter option (to do an in-place true division) doesn't seem feasible, at least when the dividend (the l.h.s. of the /= operator) has an integer dtype; to do that would require the array's dtype to be changed (which will break the contract the numpy arrays can't change their dtype), and would then also require the array memory to be reallocated to match (defeating the purpose of an in-place operation).

I do agree the operation should raise an exception in this case though -- it would keep the behavior consistent with numpy and prevent accidental misuse / incorrect results.

jack-pappas avatar Jan 15 '21 18:01 jack-pappas

That makes sense. Raising an exception is fine, and much better than the current behavior.

MarcMassar avatar Jan 15 '21 19:01 MarcMassar