-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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-1845: [Python] Expose Decimal128Type #1348
Conversation
python/pyarrow/types.pxi
Outdated
@@ -967,7 +983,25 @@ cpdef DataType decimal(int precision, int scale=0): | |||
decimal_type : DecimalType | |||
""" | |||
cdef shared_ptr[CDataType] decimal_type | |||
decimal_type.reset(new CDecimalType(precision, scale)) | |||
decimal_type.reset(new CDecimalType(byte_width, precision, scale)) | |||
return pyarrow_wrap_data_type(decimal_type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arrow::DecimalType
is the base type now -- I don't think we should allow users to create instances of it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I simply remove this function then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and the CDecimalType
class in libarrow.pxd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, that's OK to leave
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, @cpcloud is this OK with you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 LGTM
Change-Id: I4ab257d47424c43426776e46fb46a02b5e89d7f0
Here's appveyor passing: https://ci.appveyor.com/project/xhochy/arrow/build/1.0.520 |
No description provided.