cmu-delphi / epiprocess

Tools for basic signal processing in epidemiology
https://cmu-delphi.github.io/epiprocess/
Other
13 stars 8 forks source link

`epi_slide` and `epix_slide` args #510

Closed dajmcdon closed 1 month ago

dajmcdon commented 2 months ago

I still find before/after confusing. It takes mental math and careful reading of the documentation. It's more general, but not really how our target users are likely to think.

I suggest we bring back n for the window size as well as align = c("left", "center", "right"). We can keep before/after as NULL and ensure there aren't any conflicts. Then immediately process n+align into the appropriate before+after pair.

brookslogan commented 2 months ago

Thanks for the ping. I think this fell down the stack due to some engineering priorities + new testers getting tripped up by other things. Pinging @dshemetov here in case he's interested in tackling; seems related to time type rework. [Otherwise I can take a look after my other currently-planned slide edits.]

@dajmcdon have you ever used / remember us using something that's not left/center/right aligned? I know we've used non-odd-n center-aligned things at least (e.g., before = 6, after = 7), but that's the closest I can think of.

Context/history for implementer:

dshemetov commented 2 months ago

Happy to tackle this, should be pretty simple. I don't have strong opinions on the interface here, it's all equivalent to me.

dshemetov commented 2 months ago

See #513 for a simple prototype.