search

sinanpl / OaxacaBlinder

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

Add EX results #53

Open davidskalinder opened 3 months ago

davidskalinder commented 3 months ago

This PR adds EX_a and EX_b columns to the varlevel data frame in the results. (I see that Hlavac's oaxaca package stores these in $x.) The change itself is small, but it broke a bunch of old tests where I had lazily used $varlevel without specifying any columns, so most of this PR is specifying the columns in those tests (all of which now pass for me). This PR also adds a new test to compare the new columns against the same things calculated manually.

Of course this is another soft-breaking change since it adds two columns to the results, but it shouldn't change or remove anything that's already there.

I've updated #49 and #50 to specify $varlevel columns in their tests; after that change, merging this PR into a branch with those PRs in it will create a trivial merge conflict since both PRs add tests to the end of test-oaxaca.R. @sinanpl, as with #52, I don't think this is blocking anything yet, so I'll leave it open for you to review and/or accept. Meantime I've got it merged into https://github.com/davidskalinder/OaxacaBlinder-development/tree/dev_w_viewpoint_arg, so I'll continue to use that for testing.

davidskalinder commented 3 months ago

I just added another few commits to add the E(X) results to the bootstraps tree (but not to the output of coef(), where I think it's awkward next to the results for the real components).

I also added a snapshot of the full new bootstrap results, including the E(X)s. This will cause this snapshot test to fail when it gets merged with #52, since that one adds p-values to the E(X)s that this one adds. This isn't too hard to resolve -- someone just needs to approve the new snapshot on the resulting branch.

However, this is getting to be a lot to remember when the merges happen, so @sinanpl, I'm inclined to go ahead and accept #52 and this one into dev before I do any more development. I almost certainly won't build anything new today, so if you don't want these PRs to be accepted you have at least one day to say so lol! Good chance it will be longer than that though.

davidskalinder commented 3 months ago

One more update at 91a8d6905 to include EX_gap to the results (and update the snapshot) as well.