ARROW-1839/ARROW-1871: [C++/Python] Add Decimal Parquet Read/Write Tests by cpcloud · Pull Request #1335 · apache/arrow

cpcloud · 2017-11-20T06:27:13Z

No description provided.

wesm · 2017-11-27T22:00:14Z

Rebased

cpcloud · 2017-11-27T22:29:47Z

This isn't quite ready to go I have a few more things to add.

cpcloud · 2017-12-03T18:15:47Z

@wesm This is no longer a WIP and is ready for review.

I have a test (test_decimal_roundtrip_negative_scale) that depends on https://issues.apache.org/jira/browse/PARQUET-1167. I can remove that here and add in a later PR if necessary.

cpcloud · 2017-12-03T18:18:34Z

python/pyarrow/tests/test_parquet.py

This is generating a ton of tests, I will move the scale inside the function.

wesm · 2017-12-03T21:12:53Z

I force-pushed this after PARQUET-1167 was merged so we can get a passing build

wesm

+1, OK to merge once the build is passing

wesm · 2017-12-03T21:16:47Z

cpp/src/arrow/python/builtin_convert.cc

I'm not sure why these were ever virtual. Making them non-virtual because we're already using CRTP here

Thanks, sounds good to me

wesm · 2017-12-03T21:22:49Z

cpp/src/arrow/util/decimal.h

I'm renaming this to RescaleDecimal since it might be ambiguous in the arrow namespace otherwise

wesm · 2017-12-04T14:44:42Z

There's an odd Python 2.7 failure here

cpcloud · 2017-12-04T15:43:52Z

Yep I know what it is. Putting up a fix in a bit. Pylong_check is for actual long typed values. Py3 only has int checking

Change-Id: I46a48002fd7e3df2358678d3601b23b8362e61f9

Change-Id: I3883ccbcfe23d48d855aa188af92c054644670a4

Change-Id: I82c79ceae9206efde356ea241badb7a25e59adbf

Change-Id: Ic8b0a61b79b01503bfcdb8a1688c280d75b7fd66

Change-Id: I41816f4644620ea92696858a9ca96843c80d2e0c

wesm · 2017-12-04T21:36:12Z

@cpcloud RescaleDecimal wasn't exported, so instead I made it an instance method of Decimal128 if that's cool with you

cpcloud · 2017-12-04T22:21:20Z

@wesm Yep that's fine with me. We will need this for decimal kernels pretty soon.

wesm · 2017-12-04T22:55:00Z

Unrelated Travis CI failure. I'll merge as soon as we have a green Appveyor build

wesm · 2017-12-05T03:38:57Z

Appveyor finally passing: https://ci.appveyor.com/project/ApacheSoftwareFoundation/arrow/build/1.0.4623. Merging! thanks @cpcloud for your herculean efforts on this!

cpcloud changed the title ~~ARROW-1839: [C++/Python] Add Decimal Parquet Read/Write Tests~~ WIP: ARROW-1839: [C++/Python] Add Decimal Parquet Read/Write Tests Nov 20, 2017

cpcloud force-pushed the ARROW-1839 branch from 5353add to d7239cd Compare November 21, 2017 22:15

wesm force-pushed the ARROW-1839 branch from 2c74576 to a6edf4e Compare November 27, 2017 21:59

wesm force-pushed the master branch from 5220283 to 1fd3457 Compare December 1, 2017 16:50

cpcloud force-pushed the ARROW-1839 branch from a6edf4e to 362ce51 Compare December 2, 2017 18:07

cpcloud changed the title ~~WIP: ARROW-1839: [C++/Python] Add Decimal Parquet Read/Write Tests~~ ARROW-1839: [C++/Python] Add Decimal Parquet Read/Write Tests Dec 3, 2017

cpcloud changed the title ~~ARROW-1839: [C++/Python] Add Decimal Parquet Read/Write Tests~~ ARROW-1839/ARROW-1871: [C++/Python] Add Decimal Parquet Read/Write Tests Dec 3, 2017

cpcloud commented Dec 3, 2017

View reviewed changes

wesm force-pushed the ARROW-1839 branch from ec79116 to aec7631 Compare December 3, 2017 21:12

wesm approved these changes Dec 3, 2017

View reviewed changes

cpcloud added 14 commits December 4, 2017 14:26

Add Decimal parquet tests

e519c25

Use FormatValue

801626b

Use array syntax

eff7d2b

Make sure we do not override the inferred type

d7a0aab

Add better test

45eafe7

Add test util

cd38abd

Add c++ formatting test for small number and small scale/precision

ff6eccf

Make the parquet read/write from python test more robust

853fced

Test multiple precisions and scales

cf3a5a2

Checkpoint [ci skip]

fc5412a

Formatting

e0d6080

Rename

b2ad3b7

Add rescaling

7a2de88

Rewrite infer precision and scale from python

cbb2ed7

cpcloud and others added 21 commits December 4, 2017 14:26

Move rescaling to decimal.h/cc

d4a3236

Make sure we assign to our out variable

97c0a6b

Style fixes

f235ef4

More style fixes

2d02b44

Remove debugging

98e319a

Infer the scale by looking at every Python decimal object

4296b5c

Add docs

dba8a0d

Refactor test

c7637f1

Remove inference from python objects

07c356a

Add randdecimal to util

a4b1d6b

Clean up testing

1f6339d

Remove iostream include

115fbc9

Use Python C API instead of string parsing

752e905

Clarify randdecimal contract

ade030e

docstring

afe4396

Generate a column for different scales

b6f8592

int64_t for looping

bbc14cf

Change-Id: I46a48002fd7e3df2358678d3601b23b8362e61f9

Make AppendItem methods non-virtual for CRTP inlining

accc653

Change-Id: I3883ccbcfe23d48d855aa188af92c054644670a4

Rename Rescale to RescaleDecimal

3b91e55

Change-Id: I82c79ceae9206efde356ea241badb7a25e59adbf

Add missing license header

41c447f

Change-Id: Ic8b0a61b79b01503bfcdb8a1688c280d75b7fd66

Use IsPyInteger

816ff42

cpcloud force-pushed the ARROW-1839 branch from e6608bc to 816ff42 Compare December 4, 2017 19:26

cpcloud and others added 2 commits December 4, 2017 15:12

Fix namespace

878d9a1

Make RescaleDecimal instead an instance method of Decimal128

496ec79

Change-Id: I41816f4644620ea92696858a9ca96843c80d2e0c

wesm closed this in b241eb6 Dec 5, 2017

asfimport mentioned this pull request Dec 5, 2017

[C++/Python] Add Decimal Parquet Read/Write Tests #17833

Closed

Conversation

cpcloud commented Nov 20, 2017

Uh oh!

wesm commented Nov 27, 2017

Uh oh!

cpcloud commented Nov 27, 2017

Uh oh!

cpcloud commented Dec 3, 2017

Uh oh!

cpcloud Dec 3, 2017

Choose a reason for hiding this comment

Uh oh!

wesm commented Dec 3, 2017

Uh oh!

wesm left a comment

Choose a reason for hiding this comment

Uh oh!

wesm Dec 3, 2017

Choose a reason for hiding this comment

Uh oh!

cpcloud Dec 4, 2017

Choose a reason for hiding this comment

Uh oh!

wesm Dec 3, 2017

Choose a reason for hiding this comment

Uh oh!

wesm commented Dec 4, 2017

Uh oh!

cpcloud commented Dec 4, 2017

Uh oh!

wesm commented Dec 4, 2017

Uh oh!

cpcloud commented Dec 4, 2017

Uh oh!

wesm commented Dec 4, 2017

Uh oh!

wesm commented Dec 5, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants