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

POC: Partial scales #5444

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
Draft

Conversation

teunbrand
Copy link
Collaborator

@teunbrand teunbrand commented Sep 28, 2023

This PR follows up on #4271, aiming to fix #4269.

Briefly, it is convenient to set some scale parameter, without committing to a full scale yet.
This PR introduces a <Scale> subclass, <ScalePartial>, that carries around parameters. These get merged with full scales, as soon as they're added by users, or added by default.

The end goal beyond this PR is to provide convenient scales to implement #2691, so that one can set a default palette or expand in the theme without locking in other parameters.

This PR is still a work in progress:

  • Names of functions, classes and arguments are still up for grabs.
  • Only partial position scales are implemented at the moment.
  • For partial colour/fill scales and such, might need some consideration of how to handle palettes.
  • Needs some general polish and could benefit from some scrutiny

Some details:

  • I followed Hiroaki's approach of subclassing scales instead of introducing a special class for the parameters.
  • The ScalesList$add() method is responsible for merge-order of partial/full scales.
  • <Scale> objects get a fields member, which declares parameters that can be set by the user.
  • <Scale> objects get a update_params() method that merges in the partial scales.
  • <Scale> objects get a validate() method to double check that the partial scale isn't feeding illegal parameters.
  • Before overriding a trans parameter, the limits are backtransformed to apply the new trans parameter properly.
  • If the 'first' scale is a partial scale and a full scale gets merged in, the full scale already has every parameter populated. It is deduced from the full scale's call which arguments were supplied by the user. Any parameters not explicitly set by the user are fair game for the partial scale to override. This is why I had an ulterior motive to implement Better scale messages #5343.

Some examples:

devtools::load_all("~/packages/ggplot2")
#> ℹ Loading ggplot2
#> Warning: package 'testthat' was built under R version 4.3.1

# A standard plot
p <- ggplot(mpg, aes(displ, hwy)) +
  geom_point()

# Adding a partial scale
p + scale_y(trans = "sqrt")

# Partial scales can be stacked
p + scale_y(trans = "sqrt") + scale_y(breaks = seq(15, 45, by = 5))

# When two scales declare the same parameter, the latter overrules the first
p + scale_y_continuous(name = "Highway Miles") +
  scale_y(name = "Title from partial scale")

# But other parameters are kept and not overruled
p + scale_y(name = "Highway Miles",
            breaks = c(20, 30, 40),
            labels = c("A", "B", "C")) +
  scale_y_continuous(name = "Title from full scale")

Created on 2023-09-28 with reprex v2.0.2

There are still come rough patches in this PR, in particular when it comes to error messages.
In the case below, I think it is appropriate to report the full scale in the error message:

p + scale_y(breaks = c(20, 30, 40)) + 
  scale_y_continuous(labels = LETTERS[1:5])
#> Error in `scale_y_continuous()`:
#> ! `breaks` and `labels` must have the same length

However, when there is no full scale, the partial scales gets resolved during whatever build stage feels the need to add missing or default scales. This can lead to some awkward error messages, i.e. below there is no issue with geom_point() but it is mentioned first.

p + scale_y(breaks = c(20, 30, 40)) + 
  scale_y(labels = LETTERS[1:5])
#> Error in `geom_point()`:
#> ! Problem while computing aesthetics.
#> ℹ Error occurred in the 1st layer.
#> Caused by error in `scale_y()`:
#> ! `breaks` and `labels` must have the same length

Created on 2023-09-28 with reprex v2.0.2

@teunbrand teunbrand changed the title WIP: Partial scales POC: Partial scales Dec 8, 2023
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.

Set common scale parameters outside of the scale function
1 participant