-
Notifications
You must be signed in to change notification settings - Fork 21
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
Feature missing reference dates preprocessing #106
Feature missing reference dates preprocessing #106
Conversation
Moved the whole observations block to have all required variables assigned before
We need the "group" column for internal processing. Maybe we should use a more seldom keyword.
Solving #99 (rename group variable to be more unique) for the preprocessing functions touched by this PR
To do:
|
Codecov Report
@@ Coverage Diff @@
## develop #106 +/- ##
===========================================
+ Coverage 63.16% 64.29% +1.13%
===========================================
Files 12 12
Lines 961 1042 +81
===========================================
+ Hits 607 670 +63
- Misses 354 372 +18
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (self-review)
@adrian-lison planning on merging this today as really need to get something ticked off ahead of the weekend! I think everything is pretty much doing what we want and have put quite a few tests in place to catch potential issues. Note that running the example script + README was really needed to make sure nothing had changed so at some point we really need to get some more of the code downstream of preprocessing into unit testing. If you have a chance to kick the tires a bit that would be great though potentially could wait for the next big PR (i.e redesigning the interface) which will slam through over the next few days. I am also getting quite a few requests for the missing data model (i.e 2 😆 ) so it would be cool to get this in asap. I still need to fix the merge issues and make the reorgs we discussed for the stan code but perhaps we can start planning what to do about the post-processing? |
This PR is a partial replacement for #64 and includes work on the preprocessing components of #43 (i.e missing data).