JuliaData / IndexedTables.jl

Flexible tables with ordered indices
https://juliadb.org
MIT License
121 stars 37 forks source link

make missing_indxs apply to AbstractVector #288

Closed mattwigway closed 2 years ago

mattwigway commented 2 years ago

This changes the type signature of missing_indxs to apply to an AbstractVector instead of just a Vector, so that it can apply to a DataValueVector (which is making the tests for JuliaDB fail on Julia 1.7).

I'm not sure why JuliaDB is using DataValueVectors instead of Vector{DataValue} on Julia 1.7 vs 1.6, but it is - applying rows to a chunk of a distributed table:

On 1.6:

julia> rows(c2, :y)
2-element DataValues.DataValueVector{Int64}:
 DataValue{Int64}()
 DataValue{Int64}(6)

On 1.7:

julia> rows(c2, :y)
2-element DataValues.DataValueVector{Int64}:
 DataValue{Int64}()
 DataValue{Int64}(6)

CC JuliaData/JuliaDB.jl#389

joshday commented 2 years ago

Your 1.6 and 1.7 code examples are the same, right?

Either way, this change is sensible!

codecov[bot] commented 2 years ago

Codecov Report

Merging #288 (8e1f684) into main (e49cbcf) will increase coverage by 0.03%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #288      +/-   ##
==========================================
+ Coverage   87.67%   87.71%   +0.03%     
==========================================
  Files          15       15              
  Lines        1737     1750      +13     
==========================================
+ Hits         1523     1535      +12     
- Misses        214      215       +1     
Impacted Files Coverage Δ
src/selection.jl 83.87% <100.00%> (-0.92%) :arrow_down:
src/ndsparse.jl 87.13% <0.00%> (+0.07%) :arrow_up:
src/columns.jl 90.20% <0.00%> (+0.08%) :arrow_up:
src/indexing.jl 87.24% <0.00%> (+0.08%) :arrow_up:
src/join.jl 94.00% <0.00%> (+0.08%) :arrow_up:
src/indexedtable.jl 80.62% <0.00%> (+0.10%) :arrow_up:
src/utils.jl 71.51% <0.00%> (+0.18%) :arrow_up:
src/IndexedTables.jl 85.71% <0.00%> (+2.38%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e49cbcf...8e1f684. Read the comment docs.

mattwigway commented 2 years ago

Agh, been programming for 18 years and still can't use copy/paste:

v1.7: julia> rows(c2, :y) 2-element DataValues.DataValueVector{Int64}: DataValue{Int64}() DataValue{Int64}(6)

v1.6 julia> rows(c, :y) 2-element Vector{DataValue{Int64}}: DataValue{Int64}() DataValue{Int64}(6)