Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecation for ggproto classes #5962

Open
teunbrand opened this issue Jun 28, 2024 · 1 comment
Open

Deprecation for ggproto classes #5962

teunbrand opened this issue Jun 28, 2024 · 1 comment

Comments

@teunbrand
Copy link
Collaborator

This now has come up in #3798 and #5961 and the gist of it is that we have a procedure for managing lifecycles of functions, but we don't have an equivalent for ggproto classes.

We don't use GeomCol and maybe soon GeomErrorbarh anymore in ggplot2, but they are still in use by extensions. We can see for example that GeomCol is used in {ichimoku}, {entropart}, {ggiraph}, {ggside}. or that GeomErrorbarh is used in {errors} and {ggiraph}. Having a deprecation mechanism in place allows us to discourage the use of these classes while not breaking other packages.

Generally speaking, I think there are 3 ways in which ggproto classes are used in other packages::

  1. They may be subclassed in an extension class.
  2. They may be provided as arguments in e.g. layer(geom = GeomCol, ...).
  3. They're used for accessing fields, .e.g. GeomCol$default_aes.

The question of this issue is whether we can find a satisfactory way to deprecate these old classes. I think it would be relatively straightforward to throw deprecation messages in cases (1) and (2) via either ggproto() or check_subclass(). I'm not sure (3) can be caught in an easy fashion.

@teunbrand
Copy link
Collaborator Author

teunbrand commented Aug 27, 2024

Just to jot down some notes regarding topics discussed off-github;

We should ensure symmetry at the check_subclass() (unsed in layer()) level, so that it can find classes based on constructors even if the class itself is absent. This would ensure that layer(geom = "col") will find GeomBar and layer(geom = "errorbarh") will find GeomErrorbar.

Finding a class based on the constructor name is much slower than based on the class name, so probably we should only use it as a fallback.

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

check_subclass1 <- function(x, subclass, env = parent.frame()) {
  name <- paste0(subclass, camelize(x, first = TRUE))
  obj <- find_global(name, env = env)
  obj
}

check_subclass2 <- function(x, subclass, env = parent.frame()) {
  name <- paste0(to_lower_ascii(subclass), "_", x)
  obj <- find_global(name, env, mode = "function")()
  switch(
    subclass,
    Geom = obj$geom,
    Stat = obj$stat
  )
}

bench::mark(
  check_subclass1("point", "Geom"),
  check_subclass2("point", "Geom")
)
#> # A tibble: 2 × 6
#>   expression                            min  median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr>                        <bch:t> <bch:t>     <dbl> <bch:byt>    <dbl>
#> 1 "check_subclass1(\"point\", \"Ge…  79.7µs  82.2µs    11734.        0B     2.04
#> 2 "check_subclass2(\"point\", \"Ge… 758.2µs 810.4µs     1216.     101KB    11.2

Created on 2024-08-27 with reprex v2.1.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant