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

Backward compatibility for trans field deprecation: attempt 1 #5621

Closed
wants to merge 2 commits into from

Conversation

teunbrand
Copy link
Collaborator

This PR aims to fix some reverse dependency problems.

To briefly describe the problem: people are using foo <- Scale$trans all over their code. We have removed the Scale$trans field in #5566, in favour of the Scale$transformation field. All this code is now broken.

Since trans is not a function, it is not easy to deprecate elegantly. This PR uses some active binding voodoo witchcraft to (1) redirect Scale$trans to Scale$transformation and (2) issue a deprecation message when using Scale$trans. This would allow people's code to continue functioning and, by virtue of the annoying-ness of warning messages, hopefully bully them into using the new field.

I've left this PR as a draft, since active bindings are not my bread and butter and I'd like some scrutinizing eyes on this.

A demonstration:

devtools::load_all("~/packages/ggplot2/")
#> ℹ Loading ggplot2
options(lifecycle_verbosity = "warning")

scale <- scale_x_continuous(transform = "log10")

# Correct transformation is returned as expected
scale$get_transformation()
#> Transformer: log-10 [1e-100, Inf]

# Transformation field itself is also fine
scale$transformation
#> Transformer: log-10 [1e-100, Inf]

# Trans field throws warning, but returns correct transformer
scale$trans
#> Warning: Scale$trans was deprecated in ggplot2 3.5.0.
#> ℹ Please use Scale$transformation instead.
#> Call `lifecycle::last_lifecycle_warnings()` to see where this warning was
#> generated.
#> Transformer: log-10 [1e-100, Inf]

# Even if the trans field is updated, the update is redirected to the transformation field
# Note that get_transformation() returns the updated transformer
scale$trans <- transform_sqrt()
scale$get_transformation()
#> Transformer: sqrt [0, Inf]

# However, even if the field is updated, it continues to throw warnings
scale$trans
#> Warning: Scale$trans was deprecated in ggplot2 3.5.0.
#> ℹ Please use Scale$transformation instead.
#> Call `lifecycle::last_lifecycle_warnings()` to see where this warning was
#> generated.
#> Transformer: sqrt [0, Inf]

Created on 2024-01-02 with reprex v2.0.2

@teunbrand
Copy link
Collaborator Author

Closing this in favour of #5626

@teunbrand teunbrand closed this Jan 4, 2024
@teunbrand teunbrand deleted the try_deprecate_field branch January 4, 2024 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant