Restrict Decoder to compatible types (#1276)#1277
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1277 +/- ##
==========================================
+ Coverage 83.02% 83.03% +0.01%
==========================================
Files 180 180
Lines 52269 52296 +27
==========================================
+ Hits 43394 43423 +29
+ Misses 8875 8873 -2
Continue to review full report at Codecov.
|
sunchao
left a comment
sunchao
left a comment
There was a problem hiding this comment.
LGTM.
I think the decoders currently only support decoding to T::T at the moment. Not sure whether it makes sense to have a separate API that decodes directly into Arrow array. This may be beneficial to certain types such as boolean (we can directly copy the bits instead of decode to bool). Potentially we can also avoid pad_nulls based on this.
Yeah, this was one of the motivations behind #1041 - providing the ability to decode to things that aren't
FYI I did this in #1054 |
|
I see, thanks! Will check these changes. |
4 participants
Which issue does this PR close?
Closes #1276.
Rationale for this change
See ticket
What changes are included in this PR?
Restricts decoders to the types for which they make sense
Are there any user-facing changes?
Sort of, invalid decoders will now error on construction instead of when attempting to read data from them, and this may change the error behaviour of public APIs. I'm not sure if this counts.