Should all deprecation notices use warning()?

[yutannihilation]

I wasn't aware of gg_dep() until I see #3056. During checking the usages of gg_dep(), I found there are five ways (!) to show deprecation notices:

• gg_dep()
• message()
• warning()
• stop()
• .Deprecated()

Though gg_dep() seems a good idea, maybe should we consistently use warning() (or rlang::warn()?) for deprecation notices (except when there's a good reason to use some other way)? One problem of gg_dep() is that revdep checks are usually done with the dev version (the current version + .9000), which might give the different result with the release version.

> git grep -P -i '^\s*[^#\s].*deprecated' R
R/aes.r:  warning("aes_auto() is deprecated", call. = FALSE)
R/coord-transform.r:    gg_dep("1.0.1", "`xtrans` arguments is deprecated; please use `x` instead.")
R/coord-transform.r:    gg_dep("1.0.1", "`ytrans` arguments is deprecated; please use `y` instead.")
R/facet-wrap.r:    .Deprecated("strip.position", old = "switch")
R/geom-spoke.r:  message("stat_spoke is deprecated, please use geom_spoke")
R/labeller.r:          warning("Referring to `x` is deprecated, use variable name instead",
R/labeller.r:    .Deprecated(old = "keep.as.numeric")
R/labeller.r:  is_deprecated <- all(c("variable", "value") %in% names(formals(labeller)))
R/labeller.r:  if (is_deprecated) {
R/labeller.r:      "and `value` arguments are now deprecated. See labellers documentation.",
R/layer.r:    warning("`show_guide` has been deprecated. Please use `show.legend` instead.",
R/quick-plot.r:  if (!missing(stat)) warning("`stat` is deprecated", call. = FALSE)
R/quick-plot.r:  if (!missing(position)) warning("`position` is deprecated", call. = FALSE)
R/stat-bin.r:      warning("`drop` is deprecated. Please use `pad` instead.", call. = FALSE)
R/stat-bin.r:      warning("`origin` is deprecated. Please use `boundary` instead.", call. = FALSE)
R/stat-bin.r:      warning("`right` is deprecated. Please use `closed` instead.", call. = FALSE)
R/stat-bin.r:      stop("`width` is deprecated. Do you want `geom_bar()`?", call. = FALSE)
R/theme.r:    warning("`axis.ticks.margin` is deprecated. Please set `margin` property ",
R/theme.r:    warning("`panel.margin` is deprecated. Please use `panel.spacing` property ",
R/theme.r:    warning("`panel.margin.x` is deprecated. Please use `panel.spacing.x` property ",
R/theme.r:    warning("`panel.margin` is deprecated. Please use `panel.spacing` property ",
R/utilities.r:    warning(msg, " (Deprecated; last used in version ", version, ")",

[paleolimbot]

I did some searching on this, and wasn't able to find any dependencies that use gg_dep() (although a copy appears in the R Packages book section on deprecation as an example of good deprecation code). I can't find anywhere in the tests where we test for a deprecation warning, so using gg_dep() will probably not introduce any failures. If we did want to test for deprecation warnings, we could probably use a custom expectation that forces a warning.

Just as a note, the rlang package has two (unexported) functions to signal deprecation (rlang:::signal_soft_deprecated() and rlang:::warn_deprecated()).

[yutannihilation]

two (unexported) functions

They are a bonus we can get if we follow tidyverse's lifecycle policy. I think it's a good idea, though I'm not sure what amount of work will be needed.

c.f. https://github.com/r-lib/rlang/blob/master/R/compat-lifecycle.R

[paleolimbot]

New from the Tidy Principles book that will be implemented in my latest PR: tidyverse/design#67

[hadley]

I'd suggest we deprecate gg_dep() in the next release, since @paleolimbot is about to remove the final use. Then we can switch to the emerging tidyverse conventions when they firm up a bit more (although there's probably no harm in switch now, as the API is likely to be the same).

[hadley]

I think this is now done in 020e852