beacon-biosignals / MockTableGenerators.jl

Generate realistic mock datasets as dependent Tables.jl tables
MIT License
1 stars 0 forks source link

thread through RNG argument #3

Closed kleinschmidt closed 1 year ago

kleinschmidt commented 1 year ago

this basically treats emit!, visit!, num_rows, and generate like rand, by assuming the first argument is an AbstractRNG.

fixes #2

palday commented 1 year ago

you could make this nonbreaking by adding methods for defaulting to GLOBAL_RNG

kleinschmidt commented 1 year ago

yeah but I want it to be breaking so that everyone implementing a generator has to define a method that takes the RNG. otherwise you can pretty easily get into a state where it's not actually reproducible

codecov[bot] commented 1 year ago

Codecov Report

Merging #3 (a6596cf) into main (0060093) will increase coverage by 10.25%. The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             main        #3       +/-   ##
============================================
+ Coverage   89.74%   100.00%   +10.25%     
============================================
  Files           2         2               
  Lines          39        44        +5     
============================================
+ Hits           35        44        +9     
+ Misses          4         0        -4     
Impacted Files Coverage Δ
src/generate.jl 100.00% <100.00%> (+11.11%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

kleinschmidt commented 1 year ago

It would also be nice to try this out in the wild to ensure this interface is nice to work with before merging/registering this.

I've been using this in a few beacon-internal things. I don't really see another way around #2 that doesn't involve something pretty fragile/painful; @mattBrzezinski was doing some headscratching after looking at those PRs (they're internal so not gonna link em here) but last I heard hadn't thought of anything better either.