-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Geom aesthetics based on theme #5833
Conversation
Update: added text parameters to devtools::load_all("~/packages/ggplot2/")
#> ℹ Loading ggplot2
ggplot(mtcars, aes(wt, mpg, label = rownames(mtcars))) +
geom_text() +
theme_gray(base_family = "Montserrat", base_size = 22) Created on 2024-04-17 with reprex v2.1.0 |
This is in a useable state now, with almost all geoms completing colour/fill/linewidth/family/(font)size defaults from the theme. What isn't done up to this point:
ggplot2/tests/testthat/test-theme.R Line 224 in 9243e2f
Aside from these points, I'd be happy for any feedback on the direction, so I'm un-WIP-ping this PR. |
Merge branch 'main' into theme_geom_defaults # Conflicts: # R/geom-.R # R/geom-sf.R
Small update:
devtools::load_all("~/packages/ggplot2")
#> ℹ Loading ggplot2
nc <- sf::st_read(system.file("shape/nc.shp", package = "sf"), quiet = TRUE)
ggplot(nc) +
geom_sf() +
theme(
geom = element_geom(ink = "forestgreen")
) Created on 2024-04-29 with reprex v2.1.0 |
The current status of this PR is as follows; Some reverse dependencies will need to update some of their code to deal with default aesthetics that no longer are constants. I don't think there is any more work to be done on this PR at the moment and I'd recommend merging this after review so that unforeseen issues can be reported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we land on a decision on thin+thick, vs stroke+line (or something to that accord)?
Also, we should add linetype for completeness.
I'm approving so you are not blocked going forward, but let's clear the above questions before merging
outlier.shape = NULL, | ||
outlier.size = NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remind me again why we can NULL
ify these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually pch = outlier.shape %||% data$shape
where data$shape
has been filled from GeomBoxplot$default_aes
. Not setting these to NULL
would then block the theme-inherited aesthetics from reaching the points. The logic is the same for outlier.size
.
I don't think we did, but I don't have strong feelings either way. What seems more appropriate to you?
Will do! |
…plot2 into theme_geom_defaults
'outline' or 'border' instead of 'stroke' is probably more consistent with everyday terminology. |
This comment was marked as resolved.
This comment was marked as resolved.
…plot2 into theme_geom_defaults
This PR aims to partially fix #2239 and is intended to replace #2749.
It is a work in progress as a few things might need to happen before this is viable (more at the end about this).
The main gist is that there is a
from_theme()
function that works in a similar fashion toafter_stat()
/after_scale()
/stage()
.During
Geom$use_defaults()
, this will be used for masked evaluation from a new theme element.See example below for a quick demo:
It also works from the geom defaults, but this isn't implemented in any geom yet, as a few geoms (notably
geom_sf()
) are recalcitrant to this method.Created on 2024-04-08 with reprex v2.1.0
There are a few topics for discussion that mostly mirror those in #2749.
element_geom()
areink
,paper
andaccent
. You can think ofink
andpaper
as foreground and background respectively and could be renamedfg
andbg
if that is deemed more intuitive.colour_1
,colour_2
,fill_1
,fill_2
etc as discussed in Allow default geom aesthetics to be set from theme #2749, is becausefrom_theme()
could allow you to mix colours on the spot and you wouldn't need a large number of colours to account for all the different grayscale defaults in the geoms. See also next point.GeomRect
:geom_sf()
performs typicalGeom
tasks (like setting defaults) in thesf_grob()
and would require a bit of refactoring to wire this geom up directly.