Added Decimal support to pretty-print display utility (#230) #273
Conversation
Codecov Report
@@ Coverage Diff @@
## master #273 +/- ##
==========================================
+ Coverage 82.52% 82.54% +0.02%
==========================================
Files 162 162
Lines 43672 43819 +147
==========================================
+ Hits 36040 36172 +132
- Misses 7632 7647 +15
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Hi @mgill25 this is a great start.
FYI here is some more information about what the different fields on DecimalArray mean: https://github.com/apache/arrow/blob/master/format/Schema.fbs#L180-L190
FYI @ovr as I think you had been working on a more full featured DecimalArray
I think the output should show the decimal point . being added, which I am not sure it does quite yet
There was a problem hiding this comment.
Looks great to me @mgill25 . Thank you!
@jorgecarleitao or @paddyhoran can someone double check that this looks good?
| @@ -217,6 +231,9 @@ pub fn array_value_to_string(column: &array::ArrayRef, row: usize) -> Result<Str | |||
| DataType::Float16 => make_string!(array::Float32Array, column, row), | |||
| DataType::Float32 => make_string!(array::Float32Array, column, row), | |||
| DataType::Float64 => make_string!(array::Float64Array, column, row), | |||
| DataType::Decimal(_, scale) => { | |||
| make_string_from_decimal!(array::DecimalArray, column, row, scale) | |||
There was a problem hiding this comment.
Does this need to be a macro? I think you can inline this as it's the only usage, right?
There was a problem hiding this comment.
I agree it can be a function (rather than a macro) but that can always be done as a follow on PR. I think this PR is a good step forward. 👍
Which issue does this PR close?
Closes #230 .
What changes are included in this PR?
Added support for
Decimalcolumn datatype during conversion of value to String. Removes the previous error.@alamb I made a tiny change and added a couple of tests (copied similar test data from other tests I found) within the same file just to ensure things are working correctly.
I wasn't sure what the definition of "pretty" in the pretty-printing context here would be, so I made no assumptions and just propagated whatever was being printed by default. Still just in the learning mode :) If more improvements are needed, please do let me know!