Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First draft of cluster vignette #58

Merged
merged 3 commits into from
May 16, 2024

Conversation

svteichman
Copy link
Collaborator

No description provided.

@svteichman svteichman requested a review from adw96 May 3, 2024 00:16
@svteichman
Copy link
Collaborator Author

This addresses issue #38

@adw96
Copy link
Contributor

adw96 commented May 16, 2024

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!!!!

…ulate_data` function, export this function since it is now user-facing, add unit tests for `simulate_data` and `get_sim_bs`, add documentation about cluster vignette to our github.io page
@svteichman
Copy link
Collaborator Author

@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.

@svteichman svteichman merged commit e11bbb3 into statdivlab:main May 16, 2024
4 checks passed
@svteichman svteichman deleted the cluster-vignette branch May 16, 2024 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants