You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "wirable23 (via GitHub)" <gi...@apache.org> on 2023/06/21 15:06:36 UTC
[GitHub] [arrow] wirable23 opened a new issue, #35584: [Python] Converting a python sequence of floats to an arrow integer array silently truncates even when safe=True
wirable23 opened a new issue, #35584:
URL: https://github.com/apache/arrow/issues/35584
### Describe the bug, including details regarding any error messages, version, and platform.
```
>>> a = pa.array([1.2], type=pa.int64())
>>> a
<pyarrow.lib.Int64Array object at 0x000002BD16EB9F60>
[
1
]
# The following example is kept in for historical purposes but this is a separate problem and should be
# discussed in https://github.com/apache/arrow/issues/35563
>>> pa.array([18014398509481983],type=pa.float64())
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "pyarrow\array.pxi", line 327, in pyarrow.lib.array
File "pyarrow\array.pxi", line 39, in pyarrow.lib._sequence_to_array
File "pyarrow\error.pxi", line 144, in pyarrow.lib.pyarrow_internal_check_status
File "pyarrow\error.pxi", line 100, in pyarrow.lib.check_status
pyarrow.lib.ArrowInvalid: Integer value 18014398509481983 is outside of the range exactly representable by a IEEE 754 double precision value
>>>
```
Data was rounded for double to int64 but not int64 to double.
### Component(s)
Python
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] westonpace commented on issue #35584: Creating integer array with floating value succeeds
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on issue #35584:
URL: https://github.com/apache/arrow/issues/35584#issuecomment-1553619744
Interesting, I wasn't aware there was a `safe` flag for `pa.array`. It appears we are not using it when converting from a python sequence.
```
import numpy as np
import pandas as pd
import pyarrow as pa
# From numpy respects safe
pa.array(np.array([1.2]), pa.int64()) # Fails
pa.array(np.array([1.2]), pa.int64(), safe=False) # Succeeds
pa.array(pd.Series([1.2]), pa.int64()) # Fails
pa.array(pd.Series([1.2]), pa.int64(), safe=False) # Succeeds
pa.array(pa.array([1.2]), pa.int64()) # Fails
pa.array(pa.array([1.2]), pa.int64(), safe=False) # Succeeds
pa.array([1.2], pa.int64()) # Succeeds, but probably shouldn't
pa.array([1.2], pa.int64(), safe=False) # Succeeds
```
Thus, I've changed my mind, I agree that `pa.array([1.2], type=pa.int64())` should fail. However, that's both a backwards incompatible change and probably not a trivial feature (there is a wholly separate code path for converting python sequences to arrays and it doesn't seem to have a notion of safe casting right now).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] westonpace commented on issue #35584: [Python] Creating integer array with floating value succeeds
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on issue #35584:
URL: https://github.com/apache/arrow/issues/35584#issuecomment-1572053839
> Note that, although this is indeed "reimplemented" in the pyarrow conversion code, this implementation is consistent with the casting code in C++.
And whether this "check if value is withing fixed range" is the desired behaviour, or we want to relax this to larger ints that are representable as float, that's a discussion for https://github.com/apache/arrow/issues/35563
@jorisvandenbossche
I'm not sure it is consistent. Note that the original discussion here is not about "large values that are outside the range of safely representable doubles" but about "is truncation `safe`?"
https://github.com/apache/arrow/issues/35584#issuecomment-1553619744
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] westonpace commented on issue #35584: [Python] Creating integer array with floating value succeeds
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on issue #35584:
URL: https://github.com/apache/arrow/issues/35584#issuecomment-1584935699
@jorisvandenbossche yes, I agree there are two issues here. I was hoping we could use this issue for float truncation and use #35563 for out-of-range. I will update the title of this issue to reflect this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] danepitkin commented on issue #35584: [Python] Creating integer array with floating value succeeds
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on issue #35584:
URL: https://github.com/apache/arrow/issues/35584#issuecomment-1553647341
> Interesting, I wasn't aware there was a `safe` flag for `pa.array`. It appears we are not using it when converting from a python sequence.
Relevant issue here: https://github.com/apache/arrow/issues/31857. It does indeed take a different code path when converting from python to arrow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF closed issue #35584: [Python] Converting a python sequence of floats to an arrow integer array silently truncates even when safe=True
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF closed issue #35584: [Python] Converting a python sequence of floats to an arrow integer array silently truncates even when safe=True
URL: https://github.com/apache/arrow/issues/35584
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on issue #35584: [Python] Creating integer array with floating value succeeds
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on issue #35584:
URL: https://github.com/apache/arrow/issues/35584#issuecomment-1562640684
> https://github.com/apache/arrow/issues/35584#issuecomment-1550335682 Any idea why this was implemented this way and not just converted forward and back using C++ data types? Why reimplement?
Note that, although this is indeed "reimplemented" in the pyarrow conversion code, this implementation is consistent with the casting code in C++.
And whether this "check if value is withing fixed range" is the desired behaviour, or we want to relax this to larger ints that are representable as float, that's a discussion for https://github.com/apache/arrow/issues/35563
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] wirable23 commented on issue #35584: Creating integer array with floating value succeeds
Posted by "wirable23 (via GitHub)" <gi...@apache.org>.
wirable23 commented on issue #35584:
URL: https://github.com/apache/arrow/issues/35584#issuecomment-1553476325
Thanks @westonpace for the clarification. The API for pa.array is documented to be safe by default. In the cast compute API, truncation is considered unsafe. So what is the reason that in this API, truncation is safe, but in the cast API, truncation is considered unsafe?
>>> a = pa.array([1.2])
>>> a
<pyarrow.lib.DoubleArray object at 0x000002BD16F34EE0>
[
1.2
]
>>> a.cast(pa.int64())
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "pyarrow\array.pxi", line 935, in pyarrow.lib.Array.cast
File "C:\ca2_ps_311\Lib\site-packages\pyarrow\compute.py", line 400, in cast
return call_function("cast", [arr], options, memory_pool)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "pyarrow\_compute.pyx", line 572, in pyarrow._compute.call_function
File "pyarrow\_compute.pyx", line 367, in pyarrow._compute.Function.call
File "pyarrow\error.pxi", line 144, in pyarrow.lib.pyarrow_internal_check_status
File "pyarrow\error.pxi", line 100, in pyarrow.lib.check_status
pyarrow.lib.ArrowInvalid: Float value 1.2 was truncated converting to int64
>>>
In a safe cast, the truncation is raised, but in the safe array creation, all other conditions are checked except truncation. But if this is by design, then this issue can just be closed then?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on issue #35584: [Python] Creating integer array with floating value succeeds
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on issue #35584:
URL: https://github.com/apache/arrow/issues/35584#issuecomment-1562653549
Putting some responses to the comments at https://github.com/apache/arrow/issues/35563#issuecomment-1553502384 and below here:
@zfoobar I also answered this just above: I think the "design decision" here was just to be consistent with our C++ casting behaviour (i.e. the behaviour discussed in https://github.com/apache/arrow/issues/35563).
> Nowadays, I would say it's primarily a legacy implementation that eventually should be rewritten to take advantage of the compute kernels in C++ if possible.
The python_to_arrow.cc code is certainly not legacy code, it's the actual interface between python land and arrow (C++) land, and that's something we will always need to implement in pyarrow, to be able to convert builtin Python objects to Arrow data.
The C++ compute kernels don't deal with python objects. We of course can try to reuse as much as possible of those (cast) kernels in the python->arrow implementation (eg for `IntegerScalarToDoubleSafe` we could also extract the integer, convert it to a arrow integer scalar, and then call the int->float cast kernel, so we reuse the `safe` logic of the cast kernel). But that might be a quite big refactor (and for which we would also need to evaluate the performance impact).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on issue #35584: [Python] Creating integer array with floating value succeeds
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on issue #35584:
URL: https://github.com/apache/arrow/issues/35584#issuecomment-1572096392
@westonpace we are mixing two things here, I think. The top post has two examples: float truncation and large integer outside of float range. My answer was to comments from @zfoobar about the second example (e.g. https://github.com/apache/arrow/issues/35584#issuecomment-1550335682), but the title is about the first example (making this a confusing mixed discussion).
I fully agree that the float truncation does not match the C++ casting semantics of safe by default (and that's the general underlying issue: the python_to_arrow.cc code path doesn't implement the different notions of `safe=True` vs `safe=False`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] westonpace commented on issue #35584: Creating integer array with floating value succeeds
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on issue #35584:
URL: https://github.com/apache/arrow/issues/35584#issuecomment-1553030525
The int -> double behavior is a duplicate of #35563 I believe.
The double -> int behavior is by design.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF commented on issue #35584: [Python] Converting a python sequence of floats to an arrow integer array silently truncates even when safe=True
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on issue #35584:
URL: https://github.com/apache/arrow/issues/35584#issuecomment-1601015737
We created an "umbrella" issue that includes all the issues opened and connected to the Python to array conversion following the code path in `python_to_arrow.cc`. See: https://github.com/apache/arrow/issues/36202.
To sum up:
1. The first part of the issue reported which is the float truncation: as we are starting from a list the python-sequence code path is taken (`python_to_arrow.cc`) and in case of of float -> int the conversion is unsafe by default (inconsistent with numpy code path). The separate issue to track this is: https://github.com/apache/arrow/issues/36202.
2. The second part (large integer outside of float range) also has a separate issue created (as already mentioned): https://github.com/apache/arrow/issues/35563
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] zfoobar commented on issue #35584: Creating integer array with floating value succeeds
Posted by "zfoobar (via GitHub)" <gi...@apache.org>.
zfoobar commented on issue #35584:
URL: https://github.com/apache/arrow/issues/35584#issuecomment-1550335682
Any idea why this was implemented this way and not just converted forward and back using C++ data types? Why reimplement?
```python
Status IntegerScalarToDoubleSafe(PyObject* obj, double* out) {
int64_t value = 0;
RETURN_NOT_OK(UnboxIntegerAsInt64(obj, &value));
constexpr int64_t kDoubleMax = 1LL << 53;
constexpr int64_t kDoubleMin = -(1LL << 53);
if (value < kDoubleMin || value > kDoubleMax) {
return Status::Invalid("Integer value ", value, " is outside of the range exactly",
" representable by a IEEE 754 double precision value");
}
*out = static_cast<double>(value);
return Status::OK();
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] zfoobar commented on issue #35584: Creating integer array with floating value succeeds
Posted by "zfoobar (via GitHub)" <gi...@apache.org>.
zfoobar commented on issue #35584:
URL: https://github.com/apache/arrow/issues/35584#issuecomment-1550267106
See below - doesn't this indicate a more severe issue?
18014398509481984 should be representable as a float64 without any conversion errors/rounding.
```
>>> float(18014398509481983)
1.8014398509481984e+16
>>> int(1.8014398509481984e+16)
18014398509481984
>>> a = pa.array([18014398509481984], type=pa.float64())
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "pyarrow/array.pxi", line 327, in pyarrow.lib.array
result = _sequence_to_array(obj, mask, size, type, pool, c_from_pandas)
File "pyarrow/array.pxi", line 39, in pyarrow.lib._sequence_to_array
chunked = GetResultValue(
File "pyarrow/error.pxi", line 144, in pyarrow.lib.pyarrow_internal_check_status
return check_status(status)
File "pyarrow/error.pxi", line 100, in pyarrow.lib.check_status
raise ArrowInvalid(message)
pyarrow.lib.ArrowInvalid: Integer value 18014398509481984 is outside of the range exactly representable by a IEEE 754 double precision value
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org