Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-3060: [C++] Factor out string-to-X conversion routines #2433

Closed
wants to merge 1 commit into from

Conversation

pitrou
Copy link
Member

@pitrou pitrou commented Aug 15, 2018

No description provided.

@pitrou pitrou force-pushed the ARROW-3060-string-conversion branch 2 times, most recently from 0559068 to d5fbf2d Compare August 15, 2018 17:20
@codecov-io
Copy link

codecov-io commented Aug 15, 2018

Codecov Report

Merging #2433 into master will increase coverage by 1.28%.
The diff coverage is 99.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2433      +/-   ##
==========================================
+ Coverage   85.49%   86.77%   +1.28%     
==========================================
  Files         301      241      -60     
  Lines       46273    42744    -3529     
==========================================
- Hits        39563    37093    -2470     
+ Misses       6636     5651     -985     
+ Partials       74        0      -74
Impacted Files Coverage Δ
cpp/src/arrow/type_traits.h 96.66% <ø> (ø) ⬆️
cpp/src/arrow/compute/kernels/cast.cc 90.82% <100%> (-0.72%) ⬇️
cpp/src/arrow/util/parsing.h 100% <100%> (ø)
cpp/src/arrow/util/parsing-util-test.cc 99.22% <99.22%> (ø)
rust/src/util/test_util.rs
go/arrow/array/binarybuilder.go
go/arrow/array/list.go
go/arrow/type_traits_numeric.gen.go
go/arrow/datatype_fixedwidth.go
go/arrow/array/binary.go
... and 58 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c82dfcd...67e3315. Read the comment docs.

Copy link
Member

@wesm wesm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, very nice!

};

template <class ARROW_TYPE>
class StringToFloatConverterMixin {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without knowing for sure these numeric converters could end up being hot paths; I suppose we'll find out once we can do some profiling on large datasets

} // namespace internal
} // namespace arrow

#endif // ARROW_UTIL_MEMORY_H
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened a JIRA about switching to #pragma once to avoid these sorts of typos

Fix include guard typo

Change-Id: I1a3c6af13201617c75c76cab17c8fc6417fefb30
@wesm wesm force-pushed the ARROW-3060-string-conversion branch from d5fbf2d to 67e3315 Compare August 17, 2018 23:05
@wesm wesm closed this in 8db4e10 Aug 18, 2018
@pitrou pitrou deleted the ARROW-3060-string-conversion branch August 18, 2018 09:23
stephanie-wang pushed a commit to stephanie-wang/arrow that referenced this pull request Aug 29, 2018
Author: Antoine Pitrou <antoine@python.org>

Closes apache#2433 from pitrou/ARROW-3060-string-conversion and squashes the following commits:

67e3315 <Antoine Pitrou> ARROW-3060:  Factor out string-to-X conversion routines
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants