You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Alvaro-Kothe (via GitHub)" <gi...@apache.org> on 2026/01/31 03:09:51 UTC
[PR] GH-49003: [C++] Don't consider `out_of_range` an error in float parsing [arrow]
Alvaro-Kothe opened a new pull request, #49095:
URL: https://github.com/apache/arrow/pull/49095
### Rationale for this change
This PR restores the behavior previous to version 23 for floating-point parsing on overflow and subnormal.
`fast_float` didn't assign an error code on overflow in version `3.10.1` and assigned `±Inf` on overflow and `0.0` on subnormal. With the update to version `8.1`, it started to assign `std::errc::result_out_of_range` in such cases.
### What changes are included in this PR?
Ignores `std::errc::result_out_of_range` and produce `±Inf` / `0.0` as appropriate instead of failing the conversion.
### Are these changes tested?
Yes. Created tests for overflow with positive and negative signed mantissa, and also created tests for subnormal, all of them for binary{16,32,64}.
### Are there any user-facing changes?
It's a user facing change. The CSV reader on version `libarrow==23` was assigning them as strings, while before it was parsing it as `0` or `+- inf`.
With this patch, the CSV reader in PyArrow outputs:
```python
>>> import pyarrow
>>> import pyarrow.csv
>>> import io
>>> table = pyarrow.csv.read_csv(io.BytesIO(f"data\n10E-617\n10E617\n-10E617".encode()))
>>> print(table)
pyarrow.Table
data: double
----
data: [[0,inf,-inf]]
```
Closes #49003
--
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
Re: [PR] GH-49003: [C++] Don't consider `out_of_range` an error in float parsing [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou merged PR #49095:
URL: https://github.com/apache/arrow/pull/49095
--
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
Re: [PR] GH-49003: [C++] Don't consider `out_of_range` an error in float parsing [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #49095:
URL: https://github.com/apache/arrow/pull/49095#issuecomment-3842809861
CI failure is unrelated, I'll merge. @raulcd
--
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
Re: [PR] GH-49003: [C++] Don't consider `out_of_range` an error in float parsing [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #49095:
URL: https://github.com/apache/arrow/pull/49095#issuecomment-3827288165
:warning: GitHub issue #49003 **has been automatically assigned in GitHub** to PR creator.
--
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
Re: [PR] GH-49003: [C++] Don't consider `out_of_range` an error in float parsing [arrow]
Posted by "Alvaro-Kothe (via GitHub)" <gi...@apache.org>.
Alvaro-Kothe commented on code in PR #49095:
URL: https://github.com/apache/arrow/pull/49095#discussion_r2750117362
##########
cpp/src/arrow/util/value_parsing.cc:
##########
@@ -35,15 +35,17 @@ bool StringToFloat(const char* s, size_t length, char decimal_point, float* out)
::arrow_vendored::fast_float::chars_format::general, decimal_point};
const auto res =
::arrow_vendored::fast_float::from_chars_advanced(s, s + length, *out, options);
- return res.ec == std::errc() && res.ptr == s + length;
+ return (res.ec == std::errc() || res.ec == std::errc::result_out_of_range) &&
Review Comment:
Definitely it's better. I also took the liberty to store the pointer comparison in an auxiliary variable in favor of readability.
--
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
Re: [PR] GH-49003: [C++] Don't consider `out_of_range` an error in float parsing [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #49095:
URL: https://github.com/apache/arrow/pull/49095#discussion_r2759613897
##########
cpp/src/arrow/util/value_parsing_test.cc:
##########
@@ -141,6 +141,9 @@ TEST(StringConversion, ToFloat) {
AssertConversion<FloatType>("0", 0.0f);
AssertConversion<FloatType>("-0.0", -0.0f);
AssertConversion<FloatType>("-1e20", -1e20f);
+ AssertConversion<FloatType>("4e38", std::numeric_limits<float>::infinity());
+ AssertConversion<FloatType>("-4e38", -std::numeric_limits<float>::infinity());
+ AssertConversion<FloatType>("1e-46", 0.0f);
Review Comment:
Can you also test negative zeros?
--
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
Re: [PR] GH-49003: [C++] Don't consider `out_of_range` an error in float parsing [arrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #49095:
URL: https://github.com/apache/arrow/pull/49095#discussion_r2750012822
##########
cpp/src/arrow/util/value_parsing.cc:
##########
@@ -35,15 +35,17 @@ bool StringToFloat(const char* s, size_t length, char decimal_point, float* out)
::arrow_vendored::fast_float::chars_format::general, decimal_point};
const auto res =
::arrow_vendored::fast_float::from_chars_advanced(s, s + length, *out, options);
- return res.ec == std::errc() && res.ptr == s + length;
+ return (res.ec == std::errc() || res.ec == std::errc::result_out_of_range) &&
Review Comment:
Nit on readability - I think it would help to break up this line. Something like:
```cpp
const auto is_allowed_ec = res.ec == std::errc() || res.ec == std::errc::result_out_of_range;
return is_allowed_ec && res.ptr = s + length;
```
--
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
Re: [PR] GH-49003: [C++] Don't consider `out_of_range` an error in float parsing [arrow]
Posted by "Alvaro-Kothe (via GitHub)" <gi...@apache.org>.
Alvaro-Kothe commented on code in PR #49095:
URL: https://github.com/apache/arrow/pull/49095#discussion_r2759828202
##########
cpp/src/arrow/util/value_parsing_test.cc:
##########
@@ -141,6 +141,9 @@ TEST(StringConversion, ToFloat) {
AssertConversion<FloatType>("0", 0.0f);
AssertConversion<FloatType>("-0.0", -0.0f);
AssertConversion<FloatType>("-1e20", -1e20f);
+ AssertConversion<FloatType>("4e38", std::numeric_limits<float>::infinity());
+ AssertConversion<FloatType>("-4e38", -std::numeric_limits<float>::infinity());
+ AssertConversion<FloatType>("1e-46", 0.0f);
Review Comment:
Done
--
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
Re: [PR] GH-49003: [C++] Don't consider `out_of_range` an error in float parsing [arrow]
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd commented on PR #49095:
URL: https://github.com/apache/arrow/pull/49095#issuecomment-3843215092
> CI failure is unrelated, I'll merge. @raulcd
Thanks! I've cherry-picked it into the 23.0.1 maintenance branch.
--
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