Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add glance methods and IV diagnostics #285

Merged
merged 30 commits into from
Feb 28, 2019
Merged

Add glance methods and IV diagnostics #285

merged 30 commits into from
Feb 28, 2019

Conversation

lukesonnet
Copy link
Contributor

@lukesonnet lukesonnet commented Feb 4, 2019

In response to #282, and for general compatibility, we are adding glance methods for our four classes of estimators (lm_robust, iv_robust, difference_in_means, horvitz_thompson).

This entails adding some simple design details (N, N_blocks, design_type) for some of the non-regression estimators, and adding some diagnostics for the regression estimators, most notably 2SLS IV (Wu-Hausman, first stage F test, Sargan).

This has all been implemented, the diagnostics have been tested against ivreg and stata when possible, and sanity checks have been placed on the glance methods.

Notably, we don't return IV diagnostics with fixed_effects, warning instead, and we silently return NA for the overid tests with weights, and this is documented in ?iv_robust.

Also see issue #287 for documenting this with gt and gtsummary.

nfultz
nfultz previously approved these changes Feb 4, 2019
Copy link
Contributor

@nfultz nfultz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far so good.

@lukesonnet
Copy link
Contributor Author

Thanks for the comments Neal. Will implement tomorrow.

Note that in Slack we have decided not to return the first stage f-stats in glance.iv_robust if there are multiple endogenous regressors. See the problem posed here: tidymodels/broom#599

I will still compute the first-stage f-stats but they will only be visible in the returned object and when you print summary.iv_robust, not in glance.iv_robust.

@lukesonnet
Copy link
Contributor Author

This PR is ready to be merged pending code review and tests passing.

@lukesonnet lukesonnet requested a review from nfultz February 25, 2019 17:30
@lukesonnet lukesonnet changed the title WIP - Add glance methods and IV diagnostics Add glance methods and IV diagnostics Feb 25, 2019
nfultz
nfultz previously approved these changes Feb 25, 2019
Copy link
Contributor

@nfultz nfultz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, very nice. See comments for minor tweaks.

@lukesonnet lukesonnet requested a review from nfultz February 26, 2019 16:51
nfultz
nfultz previously approved these changes Feb 26, 2019
@lukesonnet
Copy link
Contributor Author

This is ready to be merged in. The only failure is on the branch travis, where the changes to the tests that are now in master have not been updated. You'll notice both PR tests have passed.

Please approve and we will merge.

Copy link
Member

@graemeblair graemeblair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Amazing work.

@lukesonnet lukesonnet merged commit cf3cda7 into master Feb 28, 2019
@lukesonnet lukesonnet deleted the ls/glance branch February 28, 2019 03:26
This was referenced Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants