tidyverse / ggplot2

An implementation of the Grammar of Graphics in R
https://ggplot2.tidyverse.org
Other
6.49k stars 2.02k forks source link

ggproto: Coord cannot be inherited #6133

Closed Yunuuuu closed 1 hour ago

Yunuuuu commented 3 hours ago

I don't know if this should be a valid way to change the internal ggproto value (avoid modifying the data in place)

library(ggplot2)
p <- ggplot(mtcars) +
    geom_point(aes(wt, mpg))
# I want to ensure the coordinate always has clip = "off"
if (!identical(p$coordinates$clip, "off")) {
    # to prevent from changing the input of user.
    p$coordinates <- ggproto(NULL, p$coordinates, clip = "off")
}
print(p)
#> Error in super() : node stack overflow

Created on 2024-10-11 with reprex v2.1.0 ~

teunbrand commented 2 hours ago

I've found that ggproto() sometimes has problems with self-references. If you make a temporary variable and use that to construct a new ggproto object it should work.

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

if (!identical(p$coordinates$clip, "off")) {
  old_coord <- p$coordinates
  p$coordinates <- ggproto(NULL, old_coord, clip = "off")
}
print(p)

Created on 2024-10-10 with reprex v2.1.1

clauswilke commented 1 hour ago

Is there anything wrong with the following code? Not sure why a new deep copy is needed.

library(ggplot2)
p <- ggplot(mtcars) +
  geom_point(aes(wt, mpg))
p$coordinates$clip <- "off"
p

Created on 2024-10-10 with reprex v2.0.2

teunbrand commented 1 hour ago

For everyday interactive use, I don't think there is a lot wrong with that code. However, when you're writing functions you can run into weird bugs because ggproto objects are not copy-on-modify:

library(ggplot2)

fun <- function(p) {
  p$coordinates$clip <- "off"
  p
}

main <- ggplot(mtcars) +
  geom_point(aes(wt, mpg))
main$coordinates$clip
#> [1] "on"

derived <- fun(main)

# This is not conform copy-on-modify
main$coordinates$clip
#> [1] "off"

Created on 2024-10-10 with reprex v2.1.1

Yunuuuu commented 1 hour ago

Thanks! the method provided by @teunbrand works.

@clauswilke I just don't want to change the user input, when we set the clip directly, it change the input p.

knitr::opts_knit$set(upload.fun = identity)
library(ggplot2)
p <- ggplot(mtcars) +
    geom_point(aes(wt, mpg))

# if user input the `p`
set_clip <- function(p) {
    p$coordinates$clip <- "off"
    p
}
old_clip <- p$coordinates$clip
set_clip(p)

new_clip <- p$coordinates$clip
identical(old_clip, new_clip)
#> [1] FALSE

Created on 2024-10-11 with reprex v2.1.0 ~
~
~

clauswilke commented 1 hour ago

I see. Yes, if you need a new copy then you need to make a copy.

teunbrand commented 1 hour ago

The behaviour is documented in ?ggproto under 'working with ggproto classes', so I don't think there is a lot to improve here. As such, I'd like to propose to close this issue.

Yunuuuu commented 1 hour ago

It's okay for me to close this issue, thanks for your help.