statdivlab / radEmu

Other
49 stars 8 forks source link

First draft of cluster vignette #58

Closed svteichman closed 4 months ago

svteichman commented 5 months ago

This addresses issue #38

adw96 commented 4 months ago

I love it, @svteichman. Minor edits commited.

Given the the simulation code isn't really important, and it might distract the user from the point of the vignette, how do you feel about extending simulate_data to allow for simulation under random effects (absent in general)? Then, you could just make a call to simulate_data() without the whole rbinom, rnbinom rigamarole.

(It could also be an opportunity to easily address #35)

If you think it's fine as is, feel free to merge. If you think streamlining the reading experience is worth it, feel free to merge after doing this.

Overall, great work! I had the idea that since people generally prefer alternatives to nulls, that I'd set beta = 4-ish rather than zero.

PS. Sorry for the delayed review!!!!

svteichman commented 4 months ago

@adw96 Thanks for the review! I moved the simulation code over to the simulate_data function, exported that function since it is now user-facing, and added tests. Once checks all pass I'll merge this and close #35 since now simulate_data should be fully covered with testing.