Match pandas scalar result types in reductions #9717
Conversation
brandon-b-miller
added
bug
|
@brandon-b-miller can you fix the style checks? |
| def _resolve_reduction_dtype(self, reduction_op: str) -> Dtype: | ||
| return self.dtype |
There was a problem hiding this comment.
Looks like we don't need this override?
There was a problem hiding this comment.
If we don't have this, we end up picking up the one from NumericalBaseColumn.
There was a problem hiding this comment.
I think that's an indication that the definition in NumericalBaseColumn should be in NumericalColumn since that's the only case requiring special behavior. Then Decimal*Column will inherit the behavior of ColumnBase as expected.
There was a problem hiding this comment.
Oh yeah. duh
| @@ -1239,6 +1239,13 @@ def _process_for_reduction( | |||
| ) | |||
| return result_col | |||
|
|
|||
| def _resolve_reduction_dtype(self, reduction_op: str) -> Dtype: | |||
There was a problem hiding this comment.
Style nit: none of our methods follow a resolve_*() naming convention, so it feels kind of out of place. Perhaps just _reduction_result_dtype?
Codecov Report
@@ Coverage Diff @@
## branch-22.02 #9717 +/- ##
================================================
- Coverage 10.49% 10.41% -0.08%
================================================
Files 119 119
Lines 20305 20480 +175
================================================
+ Hits 2130 2134 +4
- Misses 18175 18346 +171
Continue to review full report at Codecov.
|
brandon-b-miller
added
3 - Ready for Review
|
this should be ready for another look. |
|
rerun tests |
|
@gpucibot merge |
Moving this casting logic to python and updating it so that integer sum and product operations give back an
int64and give back the original column dtype in float cases. This is a breaking change.Closes #8449