Open fhollenbach opened 7 months ago
Thanks @fhollenbach and super sorry for the slow response. I had to ignore this repo for a while and dropped the ball on your PR.
I'm not adverse to adding extra arguments, but I also think it's worth weighing the cost of extra complexity from the user side. Is the main benefit here that this leads to quicker emfx
computation times (since there's less data to cover)? Or, is it mostly about being more convenient than subsetting the emfx
object after it's been created?
Hi @grantmcdermott, now I have to apologize for the slow reply. The idea was that sometimes we might want to include all the data to estimate effects, but it may only make sense to calculate an average effect over t post-treatment periods (e.g., 12 month). At the moment you could use emfx( type = "event") and then subset for that period to get the eventtime ATTs for the t-periods. But one would then have to calculate the overall average from that (including the SE). My idea was to be able to restrict the post-treatment time period in the emfx() command so that one could get the overall average effect calculated only over t-post-treatment periods. Does that seem reasonable or did I misunderstand something?
I added a simple option to only include t post-treatment periods in the calculation of aggregate effects (this option also exists in did). I have tested it a little but not 100% yet. Wanted to provide option to add this to general package.