007 - Maintaining broom tidier methods

[simonpcouch]

cc @topepo @tracykteal

[simonpcouch]

cc --

others from the tidymodels team: @juliasilge, @DavisVaughan, @hfrick, @EmilHvitfeldt broom co-authors/contributors: @dgrtwo, @alexpghayes, @grantmcdermott, @vincentarelbundock

We're introducing maintenance guidelines for broom to clarify the scope of the package and its relation to other tidier- and model-supplying packages. The changes in this PR describe the rationale and our current draft of the guidelines.

If you have a moment to read these changes over and offer any feedback you may have, it'd be very much appreciated!

I'll leave this PR open for a week (until Wednesday the 29th) and make revisions with an eye for discussion here before the upcoming 1.0.0 release.🙂

EDIT: a more readable view of the doc here

[juliasilge]

I think this is super clear and helpful, and that having this approach more transparent (rather than "unsaid") is a great move.

My one piece of feedback is for the footnote. The second example (on ANOVA) looks like it was implemented, which is a bit confusing in this context. Can we either find a different example that is more clearly something that broom would not support (and hasn't)? Or maybe in the footnote more clearly say something like:

See tidymodels/broom#1102 and tidymodels/broom#1090 for examples of broom failures that we will not address moving forward

[vincentarelbundock]

Thanks a lot for tagging me, I really appreciate it!

Like Julia, I think it's super important to have the new approach be stated explicitly and transparently. Well done!

Overall, I think the text is very clear. The only thing I don't really understand is what you mean by "visible failure." Imagine the glm() tidiers work for the binomial family but not the poisson. This wouldn't get fixed? I know it's not possible to cover all hypotheticals in a document like this, but expanding a bit on your thoughts may be useful. The rest of the main text seems excellent and clear to me..

On the general approach, let me first say that I completely understand why you want to limit the scope: A project like this could become unmanageable. In an ideal world, it would have been great to recruit a team of active maintainers. But if you were unable (or didn't want) to do that, freezing development makes sense.

That said, I do want to note that since the broom team started saying that they wanted modelling package maintainers to supply their own tidiers, I have sent working tidy and glance methods to 3 or 4 package maintainers. So far, my success rate in getting those included in upstream packages is 0%.

Maybe someone with more "clout" would have more success. And maybe a clear signal that broom is no longer developed will help. Not sure.

FWIW, the reason I made (minor) contributions to broom was that I maintain modelsummary, one of the billion regression table packages for R. I used to make heavy use of broom to extract parameters from models. Unfortunately, I have recently had to move away from broom because I got feature requests for new model types and I saw little hope of getting tiders accepted into broom or upstream packages. Obviously, mine is a very particular (and heavy) use-case. broom certainly has enough market power to remain an extremely successful package among "regular" users, even if it is mostly frozen.

Again, thanks a lot for your work on the package and on this "tidyup". Communication is super important, and this should help a lot in clarifying what people can expect from broom in the future.

[grantmcdermott]

Hi all,

Thanks @simonpcouch for tagging and the clear communication! A few quick remarks:

• As the OP of the recent ANOVA PR, I second @juliasilge's comment about its inclusion in the footnote. I think it should be dropped lest it create confusion.
• My biggest concern is the same one that @vincentarelbundock raised about "visible failures". I really think we need to leave an escape hatch open, in case an existing tidier is found to fail in an unexpected way. Even just changing the wording to "...we will generally not implement..."
• In a similar vein, it would be helpful to given an explicit example of something that would be fixed. (E.g. I know I dropped the ball here—blame COVID!—but tidy.ivreg and augment.ivreg are still broken as per this thread. Under the new guidelines, I'm not sure whether I could resuscitate my stale PR and fix things...)

Stepping back, the post is very clear and helpful. However, the all-but-overt subtext is that "broom is now in maintenance mode and is not going be to developed further". I would say something like this explicitly at the top of the post and then segue into your bullet points with something like "So what does this mean for users?"

[simonpcouch]

Thank you for all of the feedback! This was really helpful for getting a better sense of what these guidelines would mean for folks and how they can guide future dev more clearly.

The version of this document that will ship with v1.0.0 is here. The gist of the changes that made it there are:

• Deletion of the "visible failures" bullet from out-of-scope maintenance. This seemed to be the largest source of confusion for folks and has little (if any) consequence for backward compatibility, so I will make time for those!
• Addition of "in scope" examples. :)

Given our timeline with CRAN, going to go ahead and close this PR. This will live as an article on the broom site rather than as a tidyup.