ggplot2 icon indicating copy to clipboard operation
ggplot2 copied to clipboard

Better way of handling parameters

Open hadley opened this issue 10 years ago • 3 comments

Currently the way we determine the parameters of a geom/stat is ugly. For example, the parameters of a geom are mostly imputed from draw_panel and draw_group, but some need to be listed explicitly in extra_args if they're used elsewhere (as in GeomTile$setup_data().

See also #1532 - na.rm also has to be special cased.

hadley avatar Jan 25 '16 16:01 hadley

Would it make sense to have a default_params field (name to be negotiated) in the ggproto geom/stat class that gets supplemented with user input and stored in Layer${stat/geom}_params?

This would make it more analogous to how default_aes works, and as Claus remarked elsewhere, that serves two purposes by analogy. The names of the default_params field can be used in layer() to mark input as inappropriate, and secondly, to actually set a default value for an argument in the draw_panel/draw_group methods.

I think it would be relatively straightforward to do, but have to think more carefully about how to do this without negatively affecting extensions. Ideally they'd declare such a field, but it seems like quite a bit to ask without any immediate benefit.

teunbrand avatar Jan 26 '23 19:01 teunbrand

Yeah, that sounds reasonable at a high level. I'd want to see a PR implementing a very minimal version though, just to think more about it before we fully commit.

hadley avatar Jan 27 '23 13:01 hadley

I encountered an issue with the way parameters are determined, specifically the heuristic that they are taken from the formal arguments of draw_panel unless these contain ... in which draw_group is used instead. This heuristic didn't work for my scenario and was confusing to debug.

I was creating a geom that was a small extension to GeomSf that looked like:

GeomSfCustom <- ggplot2::ggproto("GeomSfCustom", ggplot2::GeomSf,
  draw_panel = function(self, data, panel_params, coord, my_param = NULL, ...) {
    # transform data with some function that uses my_param
    print(my_param)
    ggplot2::GeomSf$draw_panel(data, panel_params, coord, ...)
  },
)

I was finding that my_param was always NULL even when it was given a value in layer_sf(params = ). I tried adding it to extra_params which suppressed the unused param warning, but still my_param was always stripped. What's happening is that draw_layer trims off any params that aren't returned by parameters(extra = FALSE) (bypassing extra_params): https://github.com/tidyverse/ggplot2/blob/2e649bbc8ee2778a73043b4ffd4cf90f84bf0acf/R/geom-.R#L80

and since GeomSfCustom$draw_panel contains a ... argument, parameters instead checks draw_group: https://github.com/tidyverse/ggplot2/blob/2e649bbc8ee2778a73043b4ffd4cf90f84bf0acf/R/geom-.R#L186-L190

I found a few ways to work around this but none seem ideal:

  1. copy the full argument list of GeomSf$draw_panel to GeomSfCustom$draw_panel and keep in track if ggplot2 updates, or
  2. override parameters to hard-code in the additional param even when extra = FALSE, or
  3. define an empty draw_group with the same formal arguments as draw_panel that's never called, just so that the correct args are there.

arcresu avatar Apr 14 '23 05:04 arcresu