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

Compat with Vcov 0.8 and newer FixedEffectModels versions #53

Merged
merged 5 commits into from
Dec 28, 2023

Conversation

nilshg
Copy link
Contributor

@nilshg nilshg commented Dec 4, 2023

I'm not sure that this is correct, but I've basically changed VcovDataGLM in the same way that Vcov changed in the upstream package, and made the relevant changes to functions that use this. I'm not sure I understood the dependency structure between Vcov, StatsAPI and GLFixedEffectModels so do check if this makes sense.

Unrelated changes:

  • I fixed the deprecation for cholesky!(..., Val(true)) but had to make that conditional on version as this was only introduced in 1.7.
  • I also made the x variable local in the nlreg tests at the end to get rid of the soft scope warnings.

@nilshg
Copy link
Contributor Author

nilshg commented Dec 4, 2023

I should add I see a failure on nightly but that was there before these changes so warrants a separate investigation.

Copy link

codecov bot commented Dec 4, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8c99175) 91.24% compared to head (31dac02) 91.18%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
- Coverage   91.24%   91.18%   -0.07%     
==========================================
  Files           8        8              
  Lines         914      919       +5     
==========================================
+ Hits          834      838       +4     
- Misses         80       81       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jmboehm
Copy link
Owner

jmboehm commented Dec 28, 2023

Thanks, looks good. For some reason the estimates are slightly different on Julia nightly, not sure what's going on (but agree that this is orthogonal to the PR).

This should fix #54

@jmboehm jmboehm merged commit fa9df73 into jmboehm:master Dec 28, 2023
3 of 5 checks passed
@jmboehm jmboehm mentioned this pull request Dec 28, 2023
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.

2 participants