sinanpl / OaxacaBlinder

R implementation of Oaxaca-Blinder gap decomposition
MIT License
1 stars 1 forks source link

Add p-values #52

Open davidskalinder opened 4 months ago

davidskalinder commented 4 months ago

This is a small change to add 2-tailed p-values to the results alongside the SEs and CIs. Of course that is a breaking change, but it shouldn't cause too many headaches because it's just an addition to the results.

I couldn't come up with a good test for this since there's nothing against which to compare bootstraps, but I looked at (nearly) all of the distributions in the debugger to make sure that the values in the snapshot tests were correct; so this PR puts a new column in those snapshots. Note that the test that needed rounding ignores the new p-values, which is probably for the best since otherwise it'd be quite tricky to predict how many values on each side of zero each operating system comes up with.

The p-value calculator itself (which, as I noted in the code, I adapted from https://stats.stackexchange.com/a/83038) takes an argument that can control the null hypothesis, but for now it's always called with 0 (as, I think?, it is when lm() and the like calculate their p-values). If we want to allow users to choose a different p-value (without having to calculate it themselves from the bootstrap distributions, which they can now), we can allow them to pass a new arg to OaxacaBlinderDecomp(); but this isn't implemented yet.

Also I think it's worth noting that the p-values this PR computes are true empirical p-values in the bootstrapped distribution (like how the SE estimate is a true SD of the bootstraps). This is different from the CIs, since the CIs come from quantile(), which interpolates the gaps in a distribution, like so:

quantile(0:10, 0.12345)
#> 12.345% 
#>  1.2345

Created on 2024-05-07 with reprex v2.1.0

That means that in theory (and with few enough bootstrap repetitions), there could be cases where the CI and the p-value "disagree", and the user would have to choose whether to use the empirical version or the smoothed version from quantile(). It seems like it'd be nice to document this, but I wasn't sure where to do that (we don't have much docs about the results yet I don't think?). Of course another option would be to make the CIs strictly empirical as well, but @sinanpl I don't really have a strong opinion on that. (My opinion that the p-value and SE should be strictly empirical is a bit stronger, though I suppose I could probably be convinced of that too?)

Anyway @sinanpl I've got this in my private branch already so at this stage I'll leave this open if you want to review and/or accept it. (If it starts to create merge conflicts then I might go ahead and accept it into dev.)