Open baumgold opened 5 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Comparison is base (
ac199b0
) 87.34% compared to head (78e22be
) 87.36%.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@quinnj / @ericphanson - Any questions/comments/concerns here? If not I'd like to merge and release. Thanks.
I don't really know this code well enough to be confident here. But one thing I can say is it's probably good if the PR adds a test that is failing on main, to show precisely what has been fixed (and prevent regressions)
The current implementation depends on
Arrow.default
returning a sentinel object when the value is missing. This usually works, but occasionally has problems. This PR simplifies the implementation and solves some of the problematic corner-cases.One example of a problematic corner-case is a column of lists of structs:
It's possible that users may wrap this Vector in a
SubArray
. In this caseArrow.default(::Type{<:SubArray })
actually usesArrow.default(::Type{<:AbstractVector})
which enforces that the parent-type is aVector
- this is not always the case. For example, the parent may well be of typeArrow.Struct
if the data being written was also read from an Arrow file. Certainly we could enhanceArrow.default
to return a proper type for SubArray but simpler is to remove the dependency onArrow.default
fromToStruct
.