search

JuliaAI / ScientificTypes.jl

An API for dispatching on the "scientific" type of data instead of the machine type
MIT License
96 stars 8 forks source link

Hackmat (addresses #49) #55

Closed tlienart closed 4 years ago

tlienart commented 4 years ago

This is a hack that solves the issue discussed in #49, it could be temporarily implemented while we wait to get a better way from https://github.com/JuliaData/Tables.jl/issues/122

The hack is that I don't want to add LazyArrays as a dependency, so I can't explicitly check that things are LazyArrays, but I can check their properties, and an ApplyArray has 2 properties: :f and :args, the bet is that temporarily this is enough to uniquely identify such an array, in which case a conversion to a hard Vector is made.

This solves the issue in #49 on my side and I would suggest we bring that in while waiting for a better solution? (unless there is already a better solution)

tlienart commented 4 years ago

Great, hasproperty is not defined for Julia 1... come on...

codecov-io commented 4 years ago

Codecov Report

Merging #55 into master will decrease coverage by 3.4%. The diff coverage is 96.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #55      +/-   ##
==========================================
- Coverage   93.56%   90.16%   -3.41%     
==========================================
  Files           6        6              
  Lines         171      183      +12     
==========================================
+ Hits          160      165       +5     
- Misses         11       18       +7
Impacted Files Coverage Δ
src/ScientificTypes.jl 85.71% <ø> (ø) :arrow_up:
src/conventions/mlj/finite.jl 83.78% <100%> (-16.22%) :arrow_down:
src/conventions/mlj/mlj.jl 93.1% <100%> (+0.79%) :arrow_up:
src/autotype.jl 97.87% <100%> (ø) :arrow_up:
src/tables.jl 88.46% <66.66%> (-2.85%) :arrow_down:

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 fe7e105...22bc21e. Read the comment docs.

ablaom commented 4 years ago

Agreed!!

"hasproperty"-dispatch appropriate in this case