Add rename_size=TRUE to GeomSf, GeomCrossbar, GeomPointrange, and GeomSmooth

[yutannihilation]

Fix #4883

library(ggplot2)

ns <- asNamespace("ggplot2")
geoms_names <- ls(ns, pattern = "Geom.*")
geoms <- lapply(geoms_names, \(x) get(x, ns))

# geoms that use linewidth but don't have rename_size = TRUE
idx <- vapply(geoms, \(x) {
  use_linewidth <- "linewidth" %in% x$aesthetics()
  dont_rename_size <- !isTRUE(x$rename_size)
  use_linewidth && dont_rename_size
}, logical(1L))

geoms_names[idx]
#> [1] "GeomCrossbar"   "GeomPointrange" "GeomSf"         "GeomSmooth"

^{Created on 2022-06-20 by the reprex package (v2.0.1)}

[trevorld]

Are you sure you should do rename_size=TRUE for all those geoms? In particular shouldn't geom_sf() use both size to control the size of any "points" and linewidth to control the width of any "lines"?

[yutannihilation]

In my understanding, when rename_size = TRUE, size affects both on the point size and the line width. When not, size affects only the point size. Am I wrong?

Are you sure you should do rename_size=TRUE for all those geoms?

That's my intention, but I'm not fully sure, so I'd welcome your comments :)

library(ggplot2)

pt <- sf::st_point(c(2, 4))
ln <- sf::st_linestring(rbind(c(0, 1), c(5, 4)))
d <- sf::st_sf(id = 1:2, geometry = sf::st_sfc(pt, ln))

# current main branch
ggplot(d) +
  geom_sf(size = 10)

devtools::load_all("~/GitHub/ggplot2/")
#> ℹ Loading ggplot2

# this pull request
ggplot(d) +
  geom_sf(size = 10)

^{Created on 2022-06-20 by the reprex package (v2.0.1)}

[trevorld]

You're right (I misunderstood exactly what all of what rename_size=TRUE does).

[yutannihilation]

I see. There might be better word than "rename", but I have no idea atm

[thomasp85]

Thanks - those were oversights on my part

[yutannihilation]

Thanks for confirming! I'm getting to feel we need a bit more consideration on rename_size. I'll file a new issue so let's discuss there.

[yutannihilation]

For reference, while this pull request did make size-fallback possible, the deprecation warning won't happen with Geoms that actually have both size and linewidth because size won't be in extra_param here (and extra_aes() later).

ggplot2/R/layer.r

Lines 127 to 129 in e2d704e

	extra_param <- setdiff(names(params), all)
	# Take care of size->linewidth renaming in layer params
	if (geom$rename_size && "size" %in% extra_param && !"linewidth" %in% mapped_aesthetics(mapping)) {

Only these 3 Geoms are the cases. As I don't think we can fix this nicely and this is just a temporary workaround, I decided not to file an issue for this (but feel free to do it if necessary).

library(ggplot2)

ns <- asNamespace("ggplot2")
geoms_names <- ls(ns, pattern = "Geom.*")
geoms <- lapply(geoms_names, \(x) get(x, ns))

idx <- vapply(geoms, \(x) {
  all(c("linewidth", "size") %in% x$aesthetics())
}, logical(1L))

geoms_names[idx]
#> [1] "GeomBoxplot"    "GeomPointrange" "GeomSf"

^{Created on 2022-06-21 by the reprex package (v2.0.1)}