-
Notifications
You must be signed in to change notification settings - Fork 302
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
remove finish_glance and add nobs #597
remove finish_glance and add nobs #597
Conversation
This PR does the following:
Travis gives us the OK. There are two doc-related warnings which might have been introduced by this PR. I can't quite figure out how to make them disappear. Any tips would be much appreciated.
@alexpghayes Please let me know if there's anything you'd like me to improve. |
Re the multiple I think the first one is coming from the Line 55 in ad94acd
@evalRd in Line 283 in ad94acd
R/lme4-tidiers.R
|
Thanks for looking at this. I’m not super familiar with roxygen2, so I guess my question is why the Rd file gets written with 2 value sections. The return section you point to here is attached to the tidy method. Line 55 in ad94acd
The return section (with evalRd) you point to here is attached to the glance method: Line 283 in ad94acd
This seems like a similar setup to the one used in Line 47 in ad94acd
Line 126 in ad94acd
But I don’t get a warning in that case. More generally, I’m curious to know what is considered “proper” broom documenting style. For instance, the |
Will give this a thorough look in a day or two! |
Okay, so I'm definitely on grad student time and I apologize for that! This is fantastic! Will you:
After that I'll merge! |
cool cool. I did those two things and fixed some minor conflicts against master. Also removed lme4 and nlme4 which appear to have been deprecated. Travis breaks, but two of their machines break and Here's what the tests look like on my machine: https://gist.github.com/vincentarelbundock/dee3cb1dbb3634c442f595f3ddc39310 |
Thank you so much! Will clean up breakages myself! |
Were reverse dependencies checked for this change? There are some packages that will break such as |
That was expected. This is a very good change. |
There are a couple reverse dependencies broken, yes (#862). Filing issues/PRs right now! |
This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue. |
Following up on #594, I wanted to see how much work it would be to replace
finish_glance
with individual calls tostats::BIC
et al. inside every glance method (with@evalRd return_glance
). Turned out not to be that bad (well, it was pretty bad, but I'm quite far along now). I also improved the docs in some places along the way.Testing locally is difficult for some reason, so I'm opening this WIP PR just to see how far I am to a clean change.