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

Facet to-do list #90

Closed
11 tasks done
grantmcdermott opened this issue Dec 4, 2023 · 1 comment
Closed
11 tasks done

Facet to-do list #90

grantmcdermott opened this issue Dec 4, 2023 · 1 comment

Comments

@grantmcdermott
Copy link
Owner

grantmcdermott commented Dec 4, 2023

Moved from #83.

Relevant comments:

#83 (comment) (@zeileis)

Layout: In my first attempt to add both faceting and categorical variables #12, I had just added an argument mfrow so that we could set mfrow = TRUE or mfrow = c(1, 4) etc. Maybe this would be feasible for letting the users decide whether they want square layouts or not.

Axes: It would be good to separate out the functions for drawing the panels and drawing the axes now. This might let us add the support for categorical variables from #12 that we had further discussed in #2. My suggestion was that there should be workhorse functions a la plot2_xclass_yclass_plottype(x, y, by, axes = TRUE, ...).

From reading your description above (but not studying the code in detail), I thought that it might maybe help to have accompanying functions axis2_xlass_yclass_plottype(...) that could be inserted into the workflow for drawing the axes?

#83 (comment) (@vincentarelbundock)

Personally, I would avoid putting too much burden on the formula. Faceting does feel like a different functionality which should mostly (exclusively?) be called with its own argument. Also, focusing on the facet argument eventually allow us to do more complex things like multiple variables with a two-sided formula indicating rows and columns.

The mfrow feature seems important and powerful. Is that name too generic? Will people think it does something else, like allow multiple plot2() calls? If it only applies to facet, maybe it should be facet_mfrow. I don't have a strong view...

This was referenced Dec 4, 2023
@grantmcdermott
Copy link
Owner Author

Should we allow free axis scaling/limits for individual facets?

This is possible to do. But it ends up requiring more work internally than I initially anticipated. Reason: We have to manually reset par("usr") when drawing the plot elements of each individual facet at the end of tinyplot(). If we don't reset par("usr"), then each facet's extent is determined by the final facet plot region that was drawn higher up in the code logic. (Remember: We have to draw all of the plot regions before the drawing all the plot elements, otherwise we can't get facets to play nice with grouping.)

For posterity, I just tested a MWE with a new facet.args(... free = TRUE) argument that adds two pieces of code logic:

  1. First, inside the chunk starting at l. 917.
if (nfacets > 1) {
  # ...  [existing code]
 
   if (isTRUE(facet.args[["free"]])) {
      xlim = range(split(x, facet)[[ii]])
      ylim = range(split(y, facet)[[ii]])
      if (!is.null(ymin)) ylim = range(ylim, range(split(ymin, facet)[[ii]]))
      if (!is.null(ymax)) ylim = range(ylim, range(split(ymax, facet)[[ii]]))
      frame.plot = TRUE
   }
}
  1. Second, inside the chunk starting at l. 1150
if (nfacets > 1) {
  # ...  [existing code]

   if (isTRUE(facet.args[["free"]])) {
      xlim = range(xx)
      ylim = range(yy)
      if (!is.null(yymin)) ylim = range(ylim, range(yymin))
      if (!is.null(yymax)) ylim = range(ylim, range(yymax))
      xlim = extendrange(range(mtcars$wt),f=0.04)
      par(usr = c(extendrange(xlim), extendrange(ylim)))
   }
}

This works:

pkgload::load_all("~/Documents/Projects/tinyplot")
#> ℹ Loading tinyplot

tpar(facet.bg = "grey90")

plt(
  mpg ~ wt, mtcars,
  facet = cyl ~ am,
  facet.args = list(free = TRUE), ## TEST
  main = "Free facet axes scales/limits (example)"
)

Created on 2024-03-09 with reprex v2.1.0

Buuut.... it seems fragile: I really don't like messing with par("usr") manually and I'm not sure that it's worth the effort. So, I'm marking as free axis scales as "wont support" for the moment. We can revisit if there's strong user demand for it.

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

No branches or pull requests

1 participant