cmu-delphi / epipredict

Tools for building predictive models in epidemiology.
https://cmu-delphi.github.io/epipredict/
Other
8 stars 8 forks source link

initial layer adjustments #334

Closed dsweber2 closed 2 weeks ago

dsweber2 commented 1 month ago

This is to keep the discussion on the layer additions a little bit separate from the other PR, because that is becoming unwieldy in length. The documentation is still a WIP.

dsweber2 commented 1 month ago

ok, also added in some stuff to deal with inter-geo latency, based on @lcbrooks discussion on the other PR. The option is called epi_keys_checked, and defaults to just "geo_value", but can group by any of the epi_keys

dajmcdon commented 3 weeks ago

@dsweber2 Is this the PR I'm blocking? Is there something in particular I should focus on in review?

dsweber2 commented 3 weeks ago

This one and it's parent adjustAhead are both more or less waiting on your review. Couldn't remember when you were back, probably should've been a bit more explicit and given you a summary to work with when I thought it was ready.

As far as where to focus, probably the arguments to layer_add_forecast_date, layer_add_target_date, and the arx_* functions, and the tests are where I would focus first if I were reviewing it, but that's pretty generic.

Rough summary of this PR is:

In hindsight, there are some changes I made here that I probably should've made in the other PR and rebased onto, sorry about all the as_of -> forecast_date's in the diffs.

dsweber2 commented 3 weeks ago

Checks fail.

The tests at least were running locally for me at the last PR. I'm generally not in a habit of running the full checks locally since I expect the remote to handle that (and they really slow down the feedback loop); I guess b/c this is a PR on a PR it isn't running the full checks.

Doing so, looks like its mostly things not being in the namespace. Check now passes locally, with 4 notes (mostly some local files it should ignore, the ubiquitous global * definition, and some ::: quasi-imports).

I've marked as resolved the things I thought were straightforward in being addressed, and left open things I'm still confused on or working through.