[WIP] ARROW-4491: [Python] Use stringstreams instead of std::to_string and std::stoi #3570
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #3570 +/- ##
==========================================
+ Coverage 87.82% 88.68% +0.86%
==========================================
Files 666 537 -129
Lines 82371 73408 -8963
Branches 1069 0 -1069
==========================================
- Hits 72341 65102 -7239
+ Misses 9919 8306 -1613
+ Partials 111 0 -111
Continue to review full report at Codecov.
|
pcmoritz
changed the title
xhochy
force-pushed
the
string-conversions
branch
from
eeca9d7 to
7c8b81d
Compare
Member
|
Rebased. |
pitrou
reviewed
Feb 14, 2019
| result.push_back(static_cast<int8_t>(std::stoi(type->child(i)->name()))); | ||
| std::istringstream convert(type->child(i)->name()); | ||
| int tag; | ||
| convert >> tag; |
Member
There was a problem hiding this comment.
This will have the same problem as std::stoi. Have you looked at utils/parsing.h? The various StringConverter classes there could help ;-)
| // std::to_string is locale dependent, but should be ok for small integers | ||
| type_map_[tag] = builder_->AppendChild(*child_builder, std::to_string(tag)); | ||
| std::ostringstream convert; | ||
| convert << static_cast<int>(tag); |
Member
There was a problem hiding this comment.
Like above, the << operator is also locale-dependent by default.
One way to avoid this is to call the imbue method to change to the C locale, for example stream.imbue(std::locale::classic()). Also note this isn't very fast.
pitrou
pushed a commit
that referenced
this pull request
Feb 26, 2019
…std::stoi and std::to_string @pcmoritz Asked me to continue this PR #3570, but I don't have the right to change the his repro. Author: Philipp Moritz <[email protected]> Author: Yuhong Guo <[email protected]> Author: Antoine Pitrou <[email protected]> Closes #3758 from guoyuhong/string-conversions and squashes the following commits: 6fb7a1b <Antoine Pitrou> Make arrow_python depend on Arrow dependencies, so that dependent includes are available. 4fbda2e <Yuhong Guo> Use StringConverter c7367fa <Philipp Moritz> update bb9db84 <Philipp Moritz> use stringstreams instead of std::to_string and std::stoi
Member
|
Should be superseded by PR #3758. |
QuietCraftsmanship
pushed a commit
to QuietCraftsmanship/arrow
that referenced
this pull request
Jul 7, 2025
…std::stoi and std::to_string @pcmoritz Asked me to continue this PR apache/arrow#3570, but I don't have the right to change the his repro. Author: Philipp Moritz <[email protected]> Author: Yuhong Guo <[email protected]> Author: Antoine Pitrou <[email protected]> Closes #3758 from guoyuhong/string-conversions and squashes the following commits: 6fb7a1b6b <Antoine Pitrou> Make arrow_python depend on Arrow dependencies, so that dependent includes are available. 4fbda2e96 <Yuhong Guo> Use StringConverter c7367fa04 <Philipp Moritz> update bb9db84ed <Philipp Moritz> use stringstreams instead of std::to_string and std::stoi
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.