beacon-biosignals / LegolasFlux.jl

Save Flux model weights in Legolas-powered Arrow tables
MIT License
6 stars 1 forks source link

Allow missing weights in row construction. #14

Closed femtomc closed 2 years ago

hannahilea commented 2 years ago

Test needed!

codecov[bot] commented 2 years ago

Codecov Report

Merging #14 (5fcd62a) into main (80569ab) will increase coverage by 0.24%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #14      +/-   ##
==========================================
+ Coverage   93.87%   94.11%   +0.24%     
==========================================
  Files           2        2              
  Lines          49       51       +2     
==========================================
+ Hits           46       48       +2     
  Misses          3        3              
Impacted Files Coverage Δ
src/LegolasFlux.jl 91.89% <100.00%> (+0.46%) :arrow_up:

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

femtomc commented 2 years ago

@hannahilea / @ericphanson what sort of test would work here? Would construction with missing weights be appropriate?

hannahilea commented 2 years ago

yep!

ericphanson commented 2 years ago

I think we also don't want to check in test/my_model.model.arrow

femtomc commented 2 years ago

oops, can I add that to .gitignore (?)

ericphanson commented 2 years ago

sure!

femtomc commented 2 years ago

Interesting -- I think it depends on what we want semantically. It seems like adding missing as an element fixes the issue. I'm not exactly sure if that's good or bad.

ericphanson commented 2 years ago

I think #15 might be a simpler approach

femtomc commented 2 years ago

Closing in lieu of #15.