JuliaAI / MLJScientificTypes.jl

Implementation of the MLJ scientific type convention
MIT License
17 stars 6 forks source link

Ensure adherence to Tables.jl API #61

Closed OkonSamuel closed 3 years ago

OkonSamuel commented 3 years ago
  1. For different tables types Tables.getcolumns and Tables.columnnames call underlying efficient methods. For example in the case of DataFrame table type, Tables.columnnames(X) calls propertynames(X).

Also the previous code leads to weird behaviour as shown below

# master
julia> using MLJScientificTypes, Tables

julia> X = Dict(:a=>rand(5), :b=>rand(5))

Dict{Symbol,Array{Float64,1}} with 2 entries:
  :a => [0.517339, 0.184345, 0.723217, 0.0570972, 0.86573]
  :b => [0.880208, 0.494461, 0.78951, 0.851989, 0.8329]

julia> Tables.istable(X)
true

julia> scitype(X)
ERROR: UndefRefError: access to undefined reference
Stacktrace:

# This PR
julia> using MLJScientificTypes, Tables

julia> X = Dict(:a=>rand(5), :b=>rand(5))
Dict{Symbol,Array{Float64,1}} with 2 entries:
  :a => [0.510069, 0.161878, 0.453121, 0.755805, 0.755837]
  :b => [0.804957, 0.837796, 0.255365, 0.476521, 0.934868]

julia> Tables.istable(X)
true

julia> scitype(X)
Table{AbstractArray{Continuous,1}}

So I recommend strictly adhering to the Tables.jl API.

  1. This PR also closes issue #47.
codecov-io commented 3 years ago

Codecov Report

Merging #61 (a9f76fb) into dev (08654c3) will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev      #61   +/-   ##
=======================================
  Coverage   98.72%   98.73%           
=======================================
  Files           7        7           
  Lines         236      237    +1     
=======================================
+ Hits          233      234    +1     
  Misses          3        3           
Impacted Files Coverage Δ
src/convention/scitype.jl 96.55% <100.00%> (ø)
src/schema.jl 100.00% <100.00%> (ø)

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 08654c3...a9f76fb. Read the comment docs.