How do dependent packages deal with the broom/broom.mixed split?

[hughjonesd]

The new vignette explains what to do if you want to develop your own tidy methods. But as a user of tidy methods in my own package, I am still a bit confused.

• I can't just call broom::tidy any more, so I need to import tidy. Should this be a conditional import in NAMESPACE?
• Should I be Importing modelgenerics and Suggesting broom and broom.mixed?

[alexpghayes]

1. Re-export the generics you need from modelgenerics with

#' @importFrom modelgenerics glance
#' @export
modelgenerics::glance

this means you need to Import modelgenerics. Broom also imports these same methods so you don't need to do any conditional registration.

2. There's no need to depend on broom / broom.mixed in any way unless you're using the tidiers themselves (or stuff like fix_data_frame() I suppose).

Much more information is available in the an upcoming vignette. If you have time to read this vignette and let me know any questions it leaves unanswered, I would be greatly appreciative. Later today I will add sections on:

• when or not you need to depend on broom itself, or broom.mixed
• what happens if you re-export tidy from broom instead of modelgenerics (I think this should be fine but haven't tested it yet)

[hughjonesd]

Indeed I did read that vignette and those were my unanswered questions!

I am using the tidiers themselves - that's what I mean by being a consumer. My code calls tidy and glance. At present I just write broom::tidy and broom::glance.

I am trying out the new regime on a separate git branch. What seems to work for me (using github versions of modelgenerics, broom.mixed and broom) is to call modelgenerics::tidy and modelgenerics::glance, having requireNamespace()d broom.mixed and broom. As those packages are suggested, I can then fail with good grace if they aren't installed in a relevant method. I assume the requireNamespace is necessary... I guess then they register their methods with the modelgenerics registry? Or something.

Something I don't yet know: if I requireNamespace "broom.mixed", do I need to do the same for broom? Or does broom.mixed pull that in itself?

David

[hughjonesd]

And conversely, if I pull in both broom.mixed and broom, as they define rival methods for e.g. tidy.lme, which one will win? And which one should?

On Wed, 1 Aug 2018 at 18:14, David Hugh-Jones davidhughjones@gmail.com wrote:

Indeed I did read that vignette and those were my unanswered questions!

I am using the tidiers themselves - that's what I mean by being a consumer. My code calls tidy and glance. At present I just write broom::tidy and broom::glance.

I am trying out the new regime on a separate git branch. What seems to work for me (using github versions of modelgenerics, broom.mixed and broom) is to call modelgenerics::tidy and modelgenerics::glance, having requireNamespace()d broom.mixed and broom. As those packages are suggested, I can then fail with good grace if they aren't installed in a relevant method. I assume the requireNamespace is necessary... I guess then they register their methods with the modelgenerics registry? Or something.

Something I don't yet know: if I requireNamespace "broom.mixed", do I need to do the same for broom? Or does broom.mixed pull that in itself?

David

[alexpghayes]

Ah. I haven't played around with this at all, but clearly I should. Let me talk to Dave and get back to you.

[alexpghayes]

Okay. Demonstration package in progress, but here's what I've got so far:

1. Don't import modelgenerics::tidy. Import broom::tidy instead. It's the same thing but easier to keep track of mentally.

2. broom.mixed doesn't re-export tidy(). It should. (cc @bbolker). See bbolker/broom.mixed#23 for additional details. This is just going to be a bit messy until we're all sync'd and using modelgenerics.

Depending on whetherbroom.mixed re-exports broom::tidy from the CRAN version of broom or modelgenerics::tidy you'll need to do different things.

3. Once everybody is using modelgenerics, consumer packages should be able to continue as they always have:

#' @importFrom broom tidy
#' @export
use_tidy_method_from_broom <- function(object) {
  broom::tidy(object)
}

if the method is exported from a package other than broom, you just use that package instead:

#' @importFrom anotherpkg tidy
#' @export
use_tidy_method_from_anotherpkg <- function(object) {
  anotherpkg::tidy(object)
}

4. Using broom and broom.mixed alongside each other at the moment has to work around the fact that broom.mixed doesn't re-export tidy(). This means you need to do something like:

#' @importFrom broom tidy
#' @import broom.mixed
#' @export
use_tidy_method_from_broom.mixed <- function(object) {
  # broom.mixed::tidy(object)  # doesn't work since broom.mixed doesn't export `tidy()`
  tidy(object)
}

I have no idea how to control which method (broom vs broom.mixed) gets used here. Let me look into that.

[hughjonesd]

This is helpful.

You also need to consider what to do if one doesn't know in advance where the tidy method is defined – for example, if tidying an arbitrary object passed in by the user. That is why I thought modelgenerics::tidy could be a useful front end. Importing tidy from multiple packages is not very optimal, as you either end up cluttering the user's workspace, or you do conditional importFrom, which is run only at build time. -- Sent from Gmail Mobile

[alexpghayes]

In terms of tidying arbitrary objects: I think you'll need to @import from broom, broom.mixed and other packages that support the objects you're interested in. There's been some talk of a manifest to determine which package to a method lives in, but I don't think this is necessary for package development.

All of the tidying packages should export different methods, but they should all re-export the same generic. As a result the user's workspace doesn't get cluttered with multiple tidy() generics:

library(broom)
library(dustpan)
tidy
#> function (x, ...) 
#> {
#>     UseMethod("tidy")
#> }
#> <bytecode: 0x7fd92a441ec8>
#> <environment: namespace:modelgenerics>

Created on 2018-08-01 by the reprex package (v0.2.0).

One open issue is how to deal with methods that are defined in multiple places (the most notable case being when tidiers are moved out of broom and are deprecated in broom for a release cycle yet also defined elsewhere).

cc @topepo some topics in this thread may be relevant for modelgenerics documentation

[alexpghayes]

Thanks for these comments, they're quite helpful!

[hughjonesd]

No problem. I'm still not very satisfied with the "import from everything" solution though. It would mean I have to Import all the packages in DESCRIPTION , which are only suggested at the moment. If I have them in Suggests, and load their namespaces, won't that make the methods available for the tidy generic? -- Sent from Gmail Mobile

[alexpghayes]

Gotcha. Yes, I believe that is the case. You'll need both the method and generic, but it won't matter where you pull the generic from since it'll all be modelgenerics::tidy, etc.

[hughjonesd]

OK, that's fine! Package huxtable takes this approach in huxreg.R on GitHub. Have you got a rough release date? -- Sent from Gmail Mobile

[alexpghayes]

The modelgenerics release of broom (0.7.0) is a couple months out at least. CRAN won't let us submit for at least two months, plus modelgenerics isn't on CRAN yet, and neither is broom.mixed.

[nutterb]

I've been exploring this a bit this evening. I've set up a modelgenerics branch that imports the generics from modelgenerics. It was entirely non-functional, as no tidy method could be found for anything. I suspect I may be able to get it to work if I get the right magic to import the broom namespace, but at that point, why bother importing from modelgenerics?

It's possible that @hughjonesd is being more charitable to his users than I am. I'm not too concerned about making every tidy method available when pixiedust is installed. I'm content to let users take responsibility to install whatever broom-style packages they need to make pixiedust work. I think the error message generated is sufficient to prompt adequate inquiry (though I should probably think over how to hint that there may be other tidiers in other uninstalled packages in my documentation).

# devtools::install_github("tidymodels/modelgenerics")
library(pixiedust)
#> Warning: replacing previous import 'broom::glance' by
#> 'modelgenerics::glance' when loading 'pixiedust'
#> Warning: replacing previous import 'broom::tidy' by 'modelgenerics::tidy'
#> when loading 'pixiedust'
#> Additional documentation is being constructed at http://nutterb.github.io/pixiedust/index.html

some_fit <- lm(mpg ~ qsec, data = mtcars)
class(some_fit) <- "some_weird_object"

dust(some_fit)
#> Error in UseMethod("tidy"): no applicable method for 'tidy' applied to an object of class "some_weird_object"

[hughjonesd]

Yeah, for most people importing broom seems like the right thing to do. I only don't do it because I want to keep dependencies lean, and the function that uses broom is not central to my package.

BTW hi @nutterb, great code. Reading yours inspired me to rewrite my HTML and LaTeX output, away from my previous lame "for loops everywhere" implementation.

On Thu, 2 Aug 2018 at 02:32, Benjamin notifications@github.com wrote:

I've been exploring this a bit this evening. I've set up a modelgenerics branch https://github.com/nutterb/pixiedust/tree/modelGenerics that imports the generics from modelgenerics. It was entirely non-functional, as no tidy method could be found for anything. I suspect I may be able to get it to work if I get the right magic to import the broom namespace, but at that point, why bother importing from modelgenerics?

It's possible that @hughjonesd https://github.com/hughjonesd is being more charitable to his users than I am. I'm not too concerned about making every tidy method available when pixiedust is installed. I'm content to let users take responsibility to install whatever broom-style packages they need to make pixiedust work. I think the error message generated is sufficient to prompt adequate inquiry.

devtools::install_github("tidymodels/modelgenerics")

library(pixiedust)#> Warning: replacing previous import 'broom::glance' by#> 'modelgenerics::glance' when loading 'pixiedust'#> Warning: replacing previous import 'broom::tidy' by 'modelgenerics::tidy'#> when loading 'pixiedust'#> Additional documentation is being constructed at http://nutterb.github.io/pixiedust/index.html some_fit <- lm(mpg ~ qsec, data = mtcars) class(some_fit) <- "some_weird_object"

dust(some_fit)#> Error in UseMethod("tidy"): no applicable method for 'tidy' applied to an object of class "some_weird_object"

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/tidymodels/broom/issues/464#issuecomment-409776376, or mute the thread https://github.com/notifications/unsubscribe-auth/AFjJ97Mo2rSyVuBmgPZGHm1v26bs2xYvks5uMlbDgaJpZM4VqpZ6 .

-- Sent from Gmail Mobile

[alexpghayes]

@nutterb Just cloned that branch. The issue is that you import broom::tidy() but refer to modelgenerics::tidy(). When I undid the change to line 200 of R/dust.R the dust() function ran normally for me. Can make a PR tomorrow showing what I mean.

I think the vignette I added may be confusing in that it describes how to use modelgenerics. modelgenerics should only ever be used by developers writing new tidying methods. Consumers of tidy methods should use broom as they always have, with no reference to modelgenerics. This is true for both interactive use and functions that live in packages.

[nutterb]

Oops. It seems I didn't push the last set of changes I made last night. And you're right, what I'm experiencing is that broom::tidy() is necessary.

I had also tried using importFrom modelgenerics tidy and import broom then putting tidy(...) on line 200. That failed as well.

I like the characterization of "extending the tidy generic" vs. "consumers of existing tidy methods." I'm reading over the vignettes now to see if there's a non-disruptive way to make that point.

Yeah, for most people importing broom seems like the right thing to do. I only don't do it because I want to keep dependencies lean, and the function that uses broom is not central to my package. (@hughjonesd )

This is why I had hoped that a distinct broom.base (#256) would be in the road map. I think it's possible to build a package of just the tidiers for objects in base R that imports from tibble and tidyr and only carry Suggests for things like testthat, covr, etc. I get the feeling, however, that this isn't really under consideration any more.

Unrelated to the thread, @hughjonesd, that's quite a compliment. Especially as I consider huxtable to be a much superior implementation of a similar concept.

[hughjonesd]

How funny! I think YOUR implementation is superior... well, let's not clutter the thread with compliments. -- Sent from Gmail Mobile

[nutterb]

This may have proven to be less obvious for a consuming package than I thought. For a package like pixiedust, which consumes arbitrary tidiers, I can't seem to find an obvious way to consume arbitrary tidiers. I can make it work by listing explicit packages in either Depends: or Imports:, but it seems somewhat limiting (and burdensome) to rely on a maintainer adding any number of packages that may develop additional tidiers.

I've stood up a repository (https://github.com/nutterb/broomDepends) that goes through what I've found. The README is a decent summary of my efforts. But ultimately, I had to piece together a way to find the tidy method for a given object on the search path. This requires that the extending package export the tidy generic and that the extending package be loaded via library(). Is there a better way that I'm missing?

[alexpghayes]

This is on the docket but I don't have a great answer at the moment. I know the tidyverse team has dealt with similar issues in the past -- @jimhester @hadley do you have any advice?

[hadley]

Can you please give me a brief summary of the problem/discussion?

[jimhester]

In your package (pixiedust) you just call the generic (tidy() or glance()). You don't have to do anything else.

What method gets calls depends on what other packages have been loaded at the time the generic is called. E.g. whatever package defines the tidy.lm() method (likely broom?).

If you have existing tests for these methods, add broom to Suggests: and load broom in your tests before you call the generic.

[hughjonesd]

I may be misunderstanding but can't you just import the generic and then requireNamespace all relevant packages? If a relevant method is in one of them, it will be used. If not, you can error out and suggest the user install some packages. -- Sent from Gmail Mobile

[bbolker]

small update: broom.mixed is on CRAN and re-exports tidy, glance, augment (from broom). (Version 0.2.2 is up, a new version going up soon.) I haven't switched to modelgenerics yet, I'm not really eager to do so before modelgenerics makes it to CRAN, unless that will be a big help to someone ...

[hughjonesd]

I guess modelgenerics is becoming generics, right?

On Wed, 10 Oct 2018 at 14:25, Ben Bolker notifications@github.com wrote:

small update: broom.mixed is on CRAN and re-exports tidy, glance, augment (from broom). (Version 0.2.2 is up, a new version going up soon.) I haven't switched to modelgenerics yet, I'm not really eager to do so before modelgenerics makes it to CRAN, unless that will be a big help to someone ...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tidymodels/broom/issues/464#issuecomment-428570103, or mute the thread https://github.com/notifications/unsubscribe-auth/AFjJ92kOcFnMQrNTdi5Lc9ZGeqvLmIT7ks5ujfVEgaJpZM4VqpZ6 .

[alexpghayes]

Yes, modelgenerics has been moved to r-lib/generics. I would migrate once generics is on CRAN, but there isn't a huge rush at the moment.

[tobiasgerstenberg]

what is the best way of using tidy() from broom when the broom.mixed package was loaded?

i've tried using broom::tidy() but it still refers to the tidy function from broom.mixed instead.

running this on

R version 3.6.3 (2020-02-29)
Platform: x86_64-apple-darwin15.6.0 (64-bit)
Running under: macOS Catalina 10.15.5

with

broom.mixed_0.2.6
broom_0.7.0.9000

[github-actions[bot]]

This issue has been automatically closed due to inactivity.

[github-actions[bot]]

This issue 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.