darsnack commented 2 years ago

This removes the getindex/length defaults from AbstractDataContainer. Instead, types implementing the interface should implement Base.getindex or Base.length first, and only implement getobs/numobs when the fallback definitions don't apply.

Fixes #57.

codecov-commenter commented 2 years ago

Codecov Report

Merging #59 (c0a19e3) into main (e1f4c6b) will increase coverage by 0.21%. The diff coverage is 75.00%.

@@            Coverage Diff             @@
##             main      #59      +/-   ##
==========================================
+ Coverage   88.91%   89.13%   +0.21%     
==========================================
  Files          13       13              
  Lines         415      414       -1     
==========================================
  Hits          369      369              
+ Misses         46       45       -1

Impacted Files	Coverage Δ
src/obstransform.jl	`84.09% <57.14%> (ø)`
src/batchview.jl	`76.92% <100.00%> (+0.60%)`	:arrow_up:
src/observation.jl	`86.95% <100.00%> (+1.53%)`	:arrow_up:
src/obsview.jl	`65.78% <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 e1f4c6b...c0a19e3. Read the comment docs.

lorenzoh commented 2 years ago

I support this change 👍 Makes things simpler for newcomers, makes it easier to take advantage of types from other packages that already implement indexing, and one no longer needs to import (and export) getobs and numobs everywhere. Also the same interface as torch.utils.data.Dataset and familiarity is good.

JuliaML / MLUtils.jl

Make `getindex`/`length` the default interface #59

Codecov Report