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

Could element_geom use aesthetic argument names? #6347

Open
davidhodge931 opened this issue Feb 25, 2025 · 15 comments
Open

Could element_geom use aesthetic argument names? #6347

davidhodge931 opened this issue Feb 25, 2025 · 15 comments

Comments

@davidhodge931
Copy link

element_geom looks like a cool new feature.

Was wondering if there is a reason why colour/fill etc are not used - and instead ink/paper etc are?

It'd be cool if the arguments could align with the aesthetic names.

So currently like this...

penguins |> 
  ggplot() +
  geom_boxplot(aes(x = species, y = flipper_length_mm)) +
  theme(geom.boxplot = element_geom(ink = "blue", paper = "red")) 

This would seem much more intuitive...

penguins |> 
  ggplot() +
  geom_boxplot(aes(x = species, y = flipper_length_mm)) +
  theme(geom.boxplot = element_geom(colour = "blue", fill = "red")) 

Sorry if I'm missing something

@teunbrand
Copy link
Collaborator

teunbrand commented Feb 25, 2025

There is a need to differentiate functions. ink can also be not only be (out)line colour, but is 'foreground' colour in general. Likewise paper is not always fill and instead it is more generally the background colour. To give an example, in geom_boxplot() the boxplot foreground are the box outline and whiskers, not the box fill. In contrast, in geom_area() the fill is the foreground and it has no background elements.

In the same vein, there is a difference in line functions in that isolated lines have linewidth and linetype whereas lines that are drawn as part of borders in e.g. polygons have borderwidth and bordertype. It is also why there is no size because text size has a very different context from point size. I suppose I'd generally argue that element_geom() is much more 'function oriented' than 'form oriented' whereas this orientation is inversed for layers.

@davidhodge931
Copy link
Author

Still a bit confused, but assume you guys know what you're doing

@clauswilke
Copy link
Member

@teunbrand Only noticed this now, and wanted to point out that while I agree in principle with your argument for foreground colors and background colors I think it's important to distinguish between things that take up small areas (points, lines) versus large areas (filled areas) in the foreground. You cannot normally apply the same color to a line that you would to a filled area and vice versa. The filled area needs to be much lighter/less saturated. So this may require two different types of foreground colors, one that corresponds to typical color aesthetic and one that corresponds to typical fill aesthetic.

This would also alleviate the issue with the boxplot in my opinion, as to me the filled box is foreground, but with an appropriate color for foreground fill, not foreground line drawings.

A simple way to convert a foreground line color into a foreground fill color is usually to just add some transparency, so that could be the default option when you set the ink value for lines/points.

@davidhodge931
Copy link
Author

davidhodge931 commented Feb 26, 2025

I think I understand now the reason for this naming is to provide flexibility when applying aesthetics to all geoms.

An idea is to create an extra function element_geom_aes for people that want to adjust the aesthetics using the concepts and framework that they already know

@teunbrand
Copy link
Collaborator

Let's reopen this to air some additional space for discussion.

I think it's important to distinguish between things that take up small areas (points, lines) versus large areas (filled areas) in the foreground. You cannot normally apply the same color to a line that you would to a filled area and vice versa

I'd broadly agree with this notion, but the way this is currently implemented is as mixtures between foreground and background. For example GeomArea$default_aes has from_theme(col_mix(ink, paper, 0.2)) as its fill colour. In contrast small things like lines/points have typically from_theme(ink) directly.

the filled box is foreground, but with an appropriate color for foreground fill, not foreground line drawings

I'd agree with this when fill is a true mapping that requires a legend. For neutral boxplots, I'm not entirely sure I'm convinced that the boxplot fill is foreground.

@teunbrand teunbrand reopened this Feb 26, 2025
@davidhodge931
Copy link
Author

davidhodge931 commented Feb 26, 2025

Thanks for reopening.

Are you interested in the idea of providing an element_geom_aes or equivalent, so you can do this:

penguins |> 
  ggplot() +
  geom_boxplot(aes(x = species, y = flipper_length_mm)) +
  theme(geom.boxplot = element_geom_aes(colour = "blue", fill = "red")) 

In addition to this:

penguins |> 
  ggplot() +
  geom_boxplot(aes(x = species, y = flipper_length_mm)) +
  theme(geom.boxplot = element_geom(ink = "blue", paper = "red")) 

I feel that to have both options would provide the best of both worlds: flexibility of element_geom plus simplicity/inuitiveness of element_geom_aes (or whatever it is named).

@teunbrand
Copy link
Collaborator

I'm interested in discussing what good solutions are. The current solution is just one that works and fits with preserving previous defaults, but I'm always interested in improving on that. With regards to your specific proposal about element_geom_aes(); I don't (immediately) see how it can coexist with element_geom(). The issue is not what we provide to the theme: that can be whatever we want. Rather, how the defaults of GeomBoxplot work together with the theme is something that requires some solution. Any improvement over the current situation is worth discussing, but I would like to avoid putting additional mental burden on users by having to consider more interfaces.

@clauswilke
Copy link
Member

I'd broadly agree with this notion, but the way this is currently implemented is as mixtures between foreground and background. For example GeomArea$default_aes has from_theme(col_mix(ink, paper, 0.2)) as its fill colour. In contrast small things like lines/points have typically from_theme(ink) directly.

This is reasonable, but I would propose to intermediate this with an additional variable (fill_ink, diluted_ink, thin_ink or however you want to call it) and set that variable in the theme to col_mix(ink, paper, 0.2) as the default. Then, people can either leave it as is or change as they wish.

For neutral boxplots, I'm not entirely sure I'm convinced that the boxplot fill is foreground.

I know realize there are two different cases. The default boxplot does indeed fill with the paper color. My personal preference is to fill with diluted ink. Both are valid options in my opinion. Would be nice if this could be specified in the theme. Not sure if that is possible in the current framework.

Image

@teunbrand
Copy link
Collaborator

When we merge #6285, we'll be able to do this:

devtools::load_all("~/packages/ggplot2/")
#> ℹ Loading ggplot2

ggplot(mpg, aes(drv, displ)) +
  geom_boxplot() +
  theme_light() +
  theme(
    geom.boxplot = element_geom(paper = "grey95")
  )

Created on 2025-02-27 with reprex v2.1.1

It is not directly basing the fill from ink, but it is a solution that will bring you 90% of the way there.

@clauswilke
Copy link
Member

clauswilke commented Feb 27, 2025

Ah, Ok, so yes this achieves everything I wanted. I guess the main confusion is going to be over whether it's the ink or the paper parameter that needs to be adjusted. But a simple trial-and-error can sort that out.

Now that I think about it, geom_area() may end up being more confusing, as it mixes ink and paper to arrive at the final color (if I understand correctly), so I'll have to back-calculate what ink I need to get the fill color I want.

Either way, I think having colors getting mixed in the geom is not a good idea. I like color mixing as a good default, but in designing a theme I would definitely want to override it and do my own manual mixing.

@teunbrand
Copy link
Collaborator

teunbrand commented Feb 28, 2025

Ok so if we're talking about the specifications we want out of the new theme-based defaults, we'd like:

  • Able to provide settings in the theme()
  • Default geom fills/colours should include colour mixing, but happening outside the geom
  • An opportunity for setting colour or fill directly and opt-out of the mixing

Feel free to correct me or include additional specifications if you want.

There are two possible paths I can see to adhering to these specifications.

  1. We replace element_geom(ink, paper) with element_geom(colour, fill) and populate all geom defaults when setting complete themes like theme_gray().
  2. We add (not replace) element_geom(colour, fill) and make defaults like fill = from_theme(fill %||% col_mix(ink, paper, 0.2)). We keep element_geom(fill = NULL) as a default, so that theme designers can override as they see fit.

Happy to hear alternatives if these spring to mind

@clauswilke
Copy link
Member

I would probably have a small preference for the first option, as I agree with @davidhodge931 that reasoning about color and fill is slightly simpler, but either option works I think.

With Option 1, wouldn't you also set the default fill in the theme to col_mix(ink, paper, 0.2)?

@teunbrand
Copy link
Collaborator

teunbrand commented Feb 28, 2025

With Option 1, wouldn't you also set the default fill in the theme to col_mix(ink, paper, 0.2)?

Yes, but instead of having a single element, you'd have to populate a whole bunch of them. This also has as consequence that Geom extensions, which we cannot populate from e.g. theme_gray(), would be 2nd citizens in the automatic theming.

@clauswilke
Copy link
Member

@teunbrand Clearly you have thought this through more carefully than I have. So I'll defer to your judgement. Again, the only thing that I'd like to ask is that I can override any default color mixing in the theme.

@teunbrand
Copy link
Collaborator

I appreciate the input Claus! I'll also discuss with Thomas about his views before moving on with finishing #6285.

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

3 participants