Improve datetime scales #2186
Improve datetime scales #2186
Conversation
|
|
||
| # x/y position aesthetics should use ScaleContinuousDate or | ||
| # ScaleContinuousDatetime; others use ScaleContinuous | ||
| if (all(aesthetics %in% c("x", "xmin", "xmax", "xend", "y", "ymin", "ymax", "yend"))) { |
There was a problem hiding this comment.
Not sure if this is the best way to choose the appropriate super class...
There was a problem hiding this comment.
Can you remind me why we use ScaleContinuous for the non-position scales?
There was a problem hiding this comment.
It's so the data will get rescaled correctly, I think. ScaleContinuous uses scales::rescale() to map the data values to the right place on the scale. For position scales this isn't needed because the coordinate system handles it.
|
|
||
| # x/y position aesthetics should use ScaleContinuousDate or | ||
| # ScaleContinuousDatetime; others use ScaleContinuous | ||
| if (all(aesthetics %in% c("x", "xmin", "xmax", "xend", "y", "ymin", "ymax", "yend"))) { |
There was a problem hiding this comment.
Can you remind me why we use ScaleContinuous for the non-position scales?
|
There are a few existing tests for datetime scales, mostly they check that timezones are correct. I can add some more. |
|
I just realized that with this will make colour/fill scales behave differently for datetimes than for other continuous variables because they won't be controlled by the global options |
|
Oh hmmmm, how hard would it be to respect that option? |
|
It's a little tricky because |
|
That seems like a lot of work for little gain. Let's leave off for now and wait to see if anyone notices/complains. |
* Rewrite scale_datetime and update position/size/alpha scales * Fix style for scale_(x|y)_(date|datetime) * Update fill and colour scales for datetimes * Add to NEWS.md * Test new datetime alpha/size/colour scales * Test should use scale_colour_datetime, not scale_colour_gradient
|
This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/ |
Related to #1526. This PR:
scale_datetime()todatetime_scale()and generalizes it for use with other scales besides x/y position.datetime_scale()for size, colour, fill, and a newly created alpha scale. These scales can now all takedate_breaksanddate_labelsparameters.I also tidyverse style-ified
scale_(x|y)_dateandscale_(x|y)_datetime.