One-liner ggproto methods: \(...) foo vs. \(...) { foo }

[yjunechoe]

Have you ever wondered why some one-liner ggproto methods have { while others don't?

Geom$setup_data
#> <ggproto method>
#>   <Wrapper function>
#>     function (...) 
#> setup_data(...)
#> 
#>   <Inner function (f)>
#>     function (data, params) 
#> data

Stat$setup_data
#> <ggproto method>
#>   <Wrapper function>
#>     function (...) 
#> setup_data(...)
#> 
#>   <Inner function (f)>
#>     function (data, params) 
#> {
#>     data
#> }

Probably not!

Did you know that a function requires { for trace(at) to work?

p <- ggplot(mtcars, aes(hp, mpg)) +
  geom_point()

ggtrace::inspect_args(p, Geom$setup_data) |> rlang::catch_cnd()
#> <simpleError in .makeTracedFunction(def, tracer, exit, at, print, doEdit): cannot use 'at' argument unless the function body has the form '{ ... }'>

ggtrace::inspect_args(p, Stat$setup_data) |> rlang::catch_cnd()
#> NULL

Also probably not! 🥲

Question: would this be a too selfish of a thing to PR into ggplot? 😆

(bat signal @teunbrand)

---

In the spirit of nerding out about the internals, a quick survey of one-liners with and without curly brackets

library(ggplot2)
ggprotos <- mget(ls("package:ggplot2"), asNamespace("ggplot2"), ifnotfound = list(NULL)) |>
  Filter(\(x) inherits(x, "ggproto"), x = _)
ggproto_methods_nobrackets <- lapply(ggprotos, \(x) {
  Filter(
    \(m) inherits(x[[m]], "ggproto_method") &&
      length(body(get(m, x))) == 1,
    ls(envir = x)
  )
})
sum(lengths(ggproto_methods_nobrackets))
#> [1] 19
Filter(length, ggproto_methods_nobrackets) |>
  head()
#> $Coord
#> [1] "is_free"   "is_linear"
#> 
#> $CoordCartesian
#> [1] "is_free"   "is_linear"
#> 
#> $CoordFixed
#> [1] "is_free"
#> 
#> $CoordPolar
#> [1] "aspect"  "is_free"
#> 
#> $CoordRadial
#> [1] "is_free"
#> 
#> $CoordSf
#> [1] "is_free" "labels"

ggproto_methods_bracket_oneliner <- lapply(ggprotos, \(x) {
  Filter(
    \(m) m != "super" &&
      inherits(x[[m]], "ggproto_method") &&
      length(body(get(m, x))) == 2 &&
      body(get(m, x))[[1]] == quote(`{`) &&
      is.symbol(body(get(m, x))[[2]]),
    ls(envir = x)
  )
})
sum(lengths(ggproto_methods_bracket_oneliner))
#> [1] 31
Filter(length, ggproto_methods_bracket_oneliner) |>
  head()
#> $AxisSecondary
#> [1] "make_title"
#> 
#> $Coord
#> [1] "labels"       "setup_data"   "setup_layout"
#> 
#> $CoordMap
#> [1] "train_panel_guides"
#> 
#> $CoordPolar
#> [1] "train_panel_guides"
#> 
#> $Facet
#> [1] "finish_data" "setup_data" 
#> 
#> $GeomAnnotationMap
#> [1] "handle_na"

# Methods that are implemented both ways
intersect(
  unlist(ggproto_methods_nobrackets),
  unlist(ggproto_methods_bracket_oneliner)
)
#> [1] "labels"       "setup_data"   "setup_params" "handle_na"    "transform"

[yjunechoe]

Workaround 🙃

library(ggplot2)
p <- ggplot(mtcars, aes(hp, mpg)) +
  geom_point()

ggtrace::inspect_args(p, Geom$setup_data)
#> Error in .makeTracedFunction(def, tracer, exit, at, print, doEdit): cannot use 'at' argument unless the function body has the form '{ ... }'

assign("setup_data", function(data, params) { data }, envir = Geom)
ggtrace::inspect_args(p, Geom$setup_data)
#> $data
#>      x    y PANEL group
#> 1  110 21.0     1    -1
#> 2  110 21.0     1    -1
#> 3   93 22.8     1    -1
#> 4  110 21.4     1    -1
#> 5  175 18.7     1    -1
#> 6  105 18.1     1    -1
#> 7  245 14.3     1    -1
#> 8   62 24.4     1    -1
#> 9   95 22.8     1    -1
#> 10 123 19.2     1    -1
#> 11 123 17.8     1    -1
#> 12 180 16.4     1    -1
#> 13 180 17.3     1    -1
#> 14 180 15.2     1    -1
#> 15 205 10.4     1    -1
#> 16 215 10.4     1    -1
#> 17 230 14.7     1    -1
#> 18  66 32.4     1    -1
#> 19  52 30.4     1    -1
#> 20  65 33.9     1    -1
#> 21  97 21.5     1    -1
#> 22 150 15.5     1    -1
#> 23 150 15.2     1    -1
#> 24 245 13.3     1    -1
#> 25 175 19.2     1    -1
#> 26  66 27.3     1    -1
#> 27  91 26.0     1    -1
#> 28 113 30.4     1    -1
#> 29 264 15.8     1    -1
#> 30 175 19.7     1    -1
#> 31 335 15.0     1    -1
#> 32 109 21.4     1    -1
#> 
#> $params
#> $params$na.rm
#> [1] FALSE

[yjunechoe]

Ha! A related TIL - rlang::fn_body() will insert the { if missing (vs. body()):

get("setup_data", envir = ggplot2::Geom) |>
  body()
#> data

get("setup_data", envir = ggplot2::Geom) |>
  rlang::fn_body()
#> {
#>     data
#> }

rlang::fn_body
#> function (fn = caller_fn()) 
#> {
#>     check_closure(fn)
#>     body <- body(fn)
#>     if (is_call(body, "{")) {
#>         body
#>     }
#>     else {
#>         call("{", body)
#>     }
#> }
#> <bytecode: 0x0000020b6babb118>
#> <environment: namespace:rlang>

So it seems like there's some precedent for standardizing function bodies to start with {

[teunbrand]

From the back of my head, I know there is some obscure S4 generic/method thing that depends on curly braces vs one-liners, but I don't think applies to ggproto objects.

Question: would this be a too selfish of a thing to PR into ggplot? 😆

No, just explain in an issue why it would be beneficial first, so there is a record for why this was chosen. Another good side-effect is that RStudio document outlines will list the methods (not sure about Positron). It seems you've already nailed down unit tests by searching for ggproto objects that deviate from expectation, so that'd be good to ensure it doesn't accidentally become defunct.

[yjunechoe]

Thanks Teun! Will do to make an issue first.

Another good side-effect is that RStudio document outlines will list the methods

You're right - didn't know this!!

not sure about Positron

Seems it doesn't pick up methods at all 🥲

---

there is some obscure S4 generic/method thing that depends on curly braces vs one-liners

I wonder if it has to do with the interaction of "no {" + "function just returns a variable", in which case you get a situation where body(f) becomes a symbol 🤔

f <- \() a
g <- \() { a }

mode(body(g))
#> [1] "call"
mode(body(f))
#> [1] "name"

Or even the really unfortunate situation with returning literal values:

h <- \() NULL

length(body(h))
#> [1] 0

[yjunechoe]

Ah, actually it's for a performance reason!! From adv-r :

It is bad practice to use {} in the generic as it triggers a special case that is more expensive, and generally best avoided.

# Don't do this!
setGeneric("myGeneric", function(x) {
  standardGeneric("myGeneric")
})

Seems like there's some discussion around whether that (still) holds