[FEA] Implement DecimalColumn + Scalar and add cudf.Scalars of Decimal64Dtype #7732
Conversation
brandon-b-miller
added
2 - In Progress
brandon-b-miller
added
3 - Ready for Review
isVoid
added
feature request
|
rerun tests |
1 similar comment
|
rerun tests |
Codecov Report
@@ Coverage Diff @@
## branch-0.19 #7732 +/- ##
===============================================
+ Coverage 81.86% 82.30% +0.43%
===============================================
Files 101 101
Lines 16884 17093 +209
===============================================
+ Hits 13822 14068 +246
+ Misses 3062 3025 -37
Continue to review full report at Codecov.
|
|
@shwina @isVoid @kkraus14 @rgsl888prabhu @galipremsagar I think this should be just about ready to go. |
| cdef cppclass fixed_point_scalar[T](scalar): | ||
| fixed_point_scalar() except + | ||
| fixed_point_scalar(int64_t value, | ||
| scale_type scale, | ||
| bool is_valid) except + | ||
| int64_t value() except + | ||
| # TODO: Figure out how to add an int32 overload of value() |
There was a problem hiding this comment.
Should we be using the template type T here instead of int64_t?
There was a problem hiding this comment.
The template type in practice ends up being decimal64, see decimals.pxd for the wrapper. But we don't actually want to return that - value returns an int64_t or an int32_t for decimal64 and decimal32 respectively. What we need in the long run is an overload of value that returns an int32_t, but we didn't really see an easy way of doing that in cython.
There was a problem hiding this comment.
The hacky workaround I think I've seen in the past here is to do something like int32_t value2 "value"() except +, maybe that would work?
There was a problem hiding this comment.
I think I see how that would work. I don't mind adding this, but only regret with it is that without plumbing at least some of the rest of the pieces of the decimal32 pipeline, it might be kind of hard to tell if it works or not.
There was a problem hiding this comment.
Let's punt on it for now then, not a blocker for this PR.
|
@gpucibot merge |
kkraus14
added
5 - Ready to Merge
Closes #7498 This PR adds binary comparison `eq`, `lt`, `gt`, `le`, `ge` to - [x] decimal column v. decimal column - [x] decimal column v. decimal scalar (`decimal.Decimal` and decimal `cudf.Scalar`) - [x] decimal column v. integer column (`cudf.utils.dtypes.INTEGER_TYPES`) - [x] decimal column v. integer scalar (Python ints) Other minor adds: - Supports binary ops between `cudf.DecimalColumn` and `cudf.Scalar`, where `Scalar.dtype` is `cudf.Decimal64Dtype` (follow up for #7732 ) - Short comment noting use of decimal64 in `decimals.pxd` - Adding decimal data type in `basics.rst` Authors: - Michael Wang (https://github.com/isVoid) Approvers: - Keith Kraus (https://github.com/kkraus14) - GALI PREM SAGAR (https://github.com/galipremsagar) URL: #7716
Closes #7680