Skip to content

ARROW-4885: [C++/Python] Enable Decimal parsing in CSV - #4660

Closed
emkornfield wants to merge 4 commits into
apache:masterfrom
emkornfield:decimal_csv
Closed

ARROW-4885: [C++/Python] Enable Decimal parsing in CSV#4660
emkornfield wants to merge 4 commits into
apache:masterfrom
emkornfield:decimal_csv

Conversation

@emkornfield

@emkornfield emkornfield commented Jun 23, 2019

Copy link
Copy Markdown
Contributor
  • Create a new Decimal128 converter in csv (copies some code for eliminating white space, please let me know if this should be factored out).
  • Add python unit test
  • I filed ARROW-5699 to track 2 performance enhancements:
    • Add an UnsafeAppend and use it on the Decimal128Builder
    • Avoid multiple string copies in Decimal128::FromString.

@emkornfield emkornfield changed the title ARROW-4885: [C++/Python] Enable Decimal parsing in CSV ARROW-4885: [WIP[[C++/Python] Enable Decimal parsing in CSV Jun 23, 2019
@emkornfield emkornfield changed the title ARROW-4885: [WIP[[C++/Python] Enable Decimal parsing in CSV ARROW-4885: [WIP][C++/Python] Enable Decimal parsing in CSV Jun 23, 2019
@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #4660 into master will increase coverage by 0.56%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4660      +/-   ##
==========================================
+ Coverage   88.38%   88.95%   +0.56%     
==========================================
  Files         904      713     -191     
  Lines      112252    97095   -15157     
  Branches     1420        0    -1420     
==========================================
- Hits        99216    86372   -12844     
+ Misses      12674    10723    -1951     
+ Partials      362        0     -362
Impacted Files Coverage Δ
python/pyarrow/tests/test_csv.py 98.34% <100%> (ø) ⬆️
cpp/src/arrow/csv/converter-test.cc 100% <100%> (ø) ⬆️
cpp/src/arrow/csv/converter.cc 96.72% <100%> (-1.02%) ⬇️
go/arrow/ipc/writer.go
go/arrow/math/uint64_amd64.go
go/arrow/memory/memory_avx2_amd64.go
go/arrow/ipc/file_reader.go
js/src/builder/index.ts
js/src/enum.ts
... and 187 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 ba4e0f3...86e03f9. Read the comment docs.

@emkornfield emkornfield changed the title ARROW-4885: [WIP][C++/Python] Enable Decimal parsing in CSV ARROW-4885: [C++/Python] Enable Decimal parsing in CSV Jun 24, 2019

@pitrou pitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for doing this. Just two minor points.

Comment thread cpp/src/arrow/csv/converter.cc Outdated
Comment thread cpp/src/arrow/csv/converter.cc Outdated
Comment thread cpp/src/arrow/csv/converter.cc Outdated

@pitrou pitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1, will merge when green. Thanks @emkornfield !

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.

3 participants