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

Only generate one warning about shapes #1674

Closed
hadley opened this Issue Jul 20, 2016 · 4 comments

Comments

Projects
None yet
2 participants
@hadley
Member

hadley commented Jul 20, 2016

ggplot(data = mpg) + 
  geom_point(mapping = aes(x = displ, y = hwy, shape = class))

not two!

@hadley hadley modified the milestone: v2.2.0 Jul 28, 2016

@hadley

This comment has been minimized.

Member

hadley commented Jul 28, 2016

In fact you'll get one warning for each layer + one for the legend (guide_train.legend):

ggplot(mpg, aes(x = displ, y = hwy, shape = class)) + 
  geom_point() + 
  geom_point()

Maybe DiscreteScale$map() needs to memoise the call to palette()?

@thomasp85

This comment has been minimized.

Member

thomasp85 commented Aug 2, 2016

You want to use the memoise package or brew up some solution inside the ScaleDiscrete class?

@hadley

This comment has been minimized.

Member

hadley commented Aug 2, 2016

I think do something internal - i.e. have a field that stores the palette. But I'm not sure what happens if n changes, or even if that's possible. Also I don't remember exactly the semantics of scales - I think that would also need a tweak to the clone method.

Let's chat about this on our call so I can make sure you have my vague thoughts on the problem before you start work on it.

@hadley

This comment has been minimized.

Member

hadley commented Aug 5, 2016

I think we need a cache_palette() method that records both the palette and the number of levels. If the number is different in a future call, warn, and re-generate. That will at least allow us to see if and when n differs.

@hadley hadley added in progress and removed ready labels Aug 9, 2016

thomasp85 added a commit to thomasp85/ggplot2 that referenced this issue Aug 22, 2016

Fixes tidyverse#1674
Cache palette and n

Add news bullet

thomasp85 added a commit that referenced this issue Aug 22, 2016

Merge pull request #1702 from thomasp85/feature-cache-palette
Cache palette and n in ScaleDiscrete (#1674)

@hadley hadley removed the in progress label Aug 22, 2016

@lock lock bot locked as resolved and limited conversation to collaborators Jun 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.