Closed thomasggodfrey closed 5 years ago
Great work @thomasggodfrey! Should have some comments for you by Tuesday.
Hi @thomasggodfrey,
Apologies, you'd told me you were off this week and I completely forgot about it. Hope you've had a good week!
Overall, fantastic work. The technical detail is really well explained in the comments and it's all laid out in a logical order. I have a few (hopefully fairly small) comments on it before I approve:
0_housekeeping.R
script in a separate branch that can be used to source those dates into the other scripts instead of manually defining them each time. There's nothing you need to change right now, but, once this pull request is approved, next time you make a new branch we can swap out the dates in this script with a bit of code to source in script 0.readr
and invgamma
, since you shouldn't have to run those lines again. Hopefully anyone running this in the future will recognise from the library
calls what packages they need to have installed.Wilson
functions over multiple lines? They're pretty long at the moment and it might make them a little easier to read.denom_n_hb
, but could you follow uses of group_by
with an ungroup
afterwards?alpha = 0.05
or alpha = 0.01
inside the function calls themselves, rather than defining alpha
outside? Just in case you or anyone else ever missed the alpha = 0.01
line and accidentally ran the 99% CIs with alpha = 0.05
.Hope that's all clear and doesn't seem like too much. Happy to chat about any of it once you're back.
Hi team, can you review this when you get a moment. The code 'works', although the re-shaping at the end is just a 'work-around' and needs re-doing. I have manually checked the output in Excel and the numbers match the publication. Thanks