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

Apparent Inconsistency Between Facet$setup_data and Facet$finish_data usage #2765

Closed
brodieG opened this issue Jul 19, 2018 · 1 comment
Closed

Comments

@brodieG
Copy link
Contributor

brodieG commented Jul 19, 2018

Facet$setup_data is called by Layout with all of the plot and layer data, as seen in Layout$Setup:

    data <- c(list(plot_data), data)

    # Setup facets
    self$facet_params <- self$facet$setup_params(data, self$facet$params)
    self$facet_params$plot_env <- plot_env
    data <- self$facet$setup_data(data, self$facet_params)

However, Facet$finish_data is called repeatedly with each layer data in Layout$finish_data:

  finish_data = function(self, data) {
    lapply(data, self$facet$finish_data,
      layout = self$layout,
      x_scales = self$panel_scales_x,
      y_scales = self$panel_scales_y,
      params = self$facet_params
    )
  }

Since Facet is somewhat orthogonal to the layers (or at least applies to all the layers), it feels that the treatment in setup_data is more appropriate.

It may be too late to change this, but I imagine usage of this method is rare enough that fixing it (assuming you agree this ideally would be fixed) might not be too painful in terms of breaking existing code.

@teunbrand
Copy link
Collaborator

teunbrand commented Sep 4, 2024

Thanks for this suggestion, I agree that it would make a nice symmetry in methods.
However, unless a practical need arises I don't think we should proceed this with this as it would break some extensions.

@teunbrand teunbrand closed this as not planned Won't fix, can't repro, duplicate, stale Sep 4, 2024
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

3 participants