Remove data argument from augment() signature

[alexpghayes]

Results in warnings for broom::augment.stl() and broom::augment.rowwise_df(), which don't have a data argument. Both of these functions are likely to be deprecated at some point so that we can add the data argument back, but this currently results in a WARNING that may or may not bother the CRAN people as I work on some quick patches for the broom 0.5.1 release.

Not sure if this would even make it to CRAN in time to matter, but I could at least tell them it's in the pipeline. Advice on whether or not this change really matters appreciated.

[alexpghayes]

Note: the patches are currently off of the 0.5.1 branch of broom, i.e. https://github.com/tidymodels/broom/tree/0.5.1

[hadley]

What's the motivation for this? It seems like a step in the wrong direction to me, as it implies that models have to store their data.

[alexpghayes]

I don't think it implies that models have to store their data, but rather that they have to store their predictions, or whatever observation level information they generate. broom so far has typically dealt with models that nicely map data from some input space to some response space, but there's no need for this to always be the case.

• Unsupervised dimension reduction methods like t-SNE don't allow mapping new data from the input space to the new space
• Decompositions into trend, seasonal and noise components of a time series may not apply to new data

There's a whole class of (mainly unsupervised) transformations that you can think of as being a "non-repeatable" map into some new space.

[hadley]

Then my sense would be that should be a separate generic. It feels to me like this need more discussion; it's a small technical change but it feels like a bit philosophical one.

[alexpghayes]

I hear you. However, augment() has hitherto not had a data argument in a CRAN version of broom, so unless this gets removed, it will force unannounced breaking changes and deprecations in broom, broom.mixed and sparklyr.

[hadley]

Oh if this is just reverting a new feature that's causing problems, I don't have any objections.