Open NilsEnevoldsen opened 7 years ago
I think the warning is correct in this case.
You can do reghdfe 1.foreign ...
or reghdfe price 1.foreign ...
, but adding that in absorb would just pick one factor of all the possible factors in that variable, which is not what absorb() does.
The behavior I expect would be reghdfe price, absorb(3.rep78)
absorbs an indicator for rep78 == 3
. I expect this because it seems symmetrical with the behavior of reg price 3.rep78
. Do you disagree?
The problem is that internally 3.rep78
works more like a normal regressor than as a factor variable.
Medium term I thought about adding a partial() variable that would encompass this case. Something like what I'm doing on an ivreg2 demo:
ivreg2hdfe price weight 3.rep, absorb(turn) partial(3.rep)
(On the downside, ivreg2 requires partialled-out regressors to be both on the RHS and on partial()
)
That gets tricky if I want to do, say, ivreg2hdfe price weight 3.rep, absorb(turn#3.rep)
.
Agreed. Was playing with a similar example and saw that it only accepts i. and c. prefixes. One of the reasons for being so strict is to prevent bugs in absorb() , since writing parsing commands is a real pain in the ass (and in this case, required 266 lines of code just for testing
I'll think about what can be changed to make it more flexible without risking introducing bugs.
So the code for the regressors has been streamlined and should be mostly free of issues, but the code for absorb() is still tricky: there is no way it can be treated as a normal varlist because it's not a normal varlist:
i.turn##c.gear
should not expand the ##
because that means something different (compute two different sets of FEs instead of a joint one with intercept and slope)i.turn##c.(gear weight)
(don't expand what's inside the parens)Thus, we have a lot of special cases and can't rely on syntax/fvunab/fvexpand/etc.
Since it looks like a huge hassle for little gain, I'll mark it as wontfix for now and perhaps at some point me or someone will go through the work of improving it