Skip to content

facet_wrap() and facet_grid() with a date variable is broken #3313

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

Closed
paleolimbot opened this issue May 8, 2019 · 17 comments · Fixed by #3331
Closed

facet_wrap() and facet_grid() with a date variable is broken #3313

paleolimbot opened this issue May 8, 2019 · 17 comments · Fixed by #3331
Milestone

Comments

@paleolimbot
Copy link
Member

The revdeps (#3303) identified that using facet_grid() or facet_wrap() with a date variable is broken. This probably affects other S3 vectors as well.

library(ggplot2)
df <- data.frame(date = as.Date("2019-01-01"), x = 1, y = 1)
p <- ggplot(df, aes(x, y)) + geom_point()
p + facet_grid(vars(date))
#> Error in scale_apply(layer_data, x_vars, "train", SCALE_X, x_scales):
p + facet_wrap(vars(date))
#> Error in scale_apply(layer_data, x_vars, "train", SCALE_X, x_scales):

The error is in Facet(Grid|Wrap)$map_data(), where there is a join between the layout and the panels. In one of these, date is a factor, and in the other, it is an S3 Date. This changed because of different behaviour between join_keys() and its previous plyr counterpart.

@clauswilke
Copy link
Member

@yutannihilation Do you have time to look into this issue?

@clauswilke clauswilke mentioned this issue May 10, 2019
17 tasks
@yutannihilation
Copy link
Member

Sure, will do.

@yutannihilation
Copy link
Member

Quickly confirmed #3327 fixes this issue. Sorry, I'm afraid I won't be very responsive until the end of this month...

@paleolimbot
Copy link
Member Author

I would be happy to PR a test for this if that is helpful.

@thomasp85
Copy link
Member

I don’t think it makes sense to test for some arbitrary ordering as long as we don’t break something in revdeps... we will probably have to actively break this once we move to rbinding through vctrs at is is unlikely to follow the arbitrary rules of plyr, but it is best to break only once

@yutannihilation
Copy link
Member

Though ordering is just an implementational detail, I think it makes sense to ensure facetting by Date is not broken.

@thomasp85
Copy link
Member

I’m simply weary because the old ordering (which is now replicated) is logically wrong. Setting up a unit test may give it some undeserved credibility

@yutannihilation
Copy link
Member

Yeah, I agree that we don't need a test about ordering, but the one about facetting by Date (probably it would not be a unit test).

@yutannihilation
Copy link
Member

(But, since I don't have enough time to help right now and it seems not necessary for the release, please ignore my comments. Sorry...)

@paleolimbot
Copy link
Member Author

I accidentally closed this from a typo in PR 3331...can I/should I fix this?

@yutannihilation
Copy link
Member

yutannihilation commented May 17, 2019

BTW, what about POSIXct? This still fails with #3327.

library(ggplot2)

packageDescription("ggplot2")[c("GithubRef", "GithubSHA1")]
#> $GithubRef
#> [1] "plyr-order"
#> 
#> $GithubSHA1
#> [1] "b078f6901cc66968a04587585015559a42f71087"

df <- data.frame(date = as.Date("2019-01-01"), x = 1, y = 1)
df$date <- as.POSIXct(df$date)
p <- ggplot(df, aes(x, y)) + geom_point()
p + facet_grid(vars(date))
#> Error in scale_apply(layer_data, x_vars, "train", SCALE_X, x_scales):

@thomasp85
Copy link
Member

I've fixed POSIXct... that was the last of the classes that received special treatment in plyr so hopefully this will be it for weird facet failures

@yutannihilation
Copy link
Member

Oh, thanks for fixing!

paleolimbot added a commit to paleolimbot/ggplot2 that referenced this issue May 20, 2019
@thomasp85
Copy link
Member

closed by #3327

@cf814
Copy link

cf814 commented Aug 7, 2019

This issue appears to occur still with POSIXct variables, but not with Dates - see attached sample data and below code.

outlierSummary.zip

outlierSummary$week2 <- as.Date(outlierSummary$week, format = "%Y-%m-%d")

obc=ggplot(data=outlierSummary,aes(x=type,y=outpc,fill=type))+geom_bar(stat = "identity")+
ylim(c(0,3))+geom_text(aes(label=outliers,x=type,y=outpc),position=position_dodge(width=0.9), size=4,vjust=0)

#With posixct
obc <- obc + facet_grid(week ~ period)+labs(x="Provider", y="Weekly Rate of Outliers (%)",fill="Provider", caption="Numbers above bars: Outlier counts")+theme_bw()

#With Date
obc2 <- obc + facet_grid(week2 ~ period)+labs(x="Provider", y="Weekly Rate of Outliers (%)",fill="Provider", caption="Numbers above bars: Outlier counts")+theme_bw()

@paleolimbot
Copy link
Member Author

Could you open a new issue with a more minimal example? It saves us quite a bit of time if we can copy and paste a small section of code to recreate the error.

@lock
Copy link

lock bot commented Feb 4, 2020

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/

@lock lock bot locked and limited conversation to collaborators Feb 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants