Compatibility ggplot2 400#129
Conversation
|
|
||
|
|
||
| default_aes = aes( | ||
| shape = NULL, | ||
| colour = "gray30", | ||
| fill = "gray90", | ||
| linetype = 1, | ||
| stroke = 0.5, | ||
| size = 3.88, | ||
| !!!GeomSf$default_aes[-which(names(GeomSf$default_aes) %in% c("size", "linewidth"))], | ||
| !!!GeomText$default_aes[c("family", "size")], | ||
| linewidth = !!GeomLine$default_aes$linewidth, |
There was a problem hiding this comment.
The new ggplot2 version has more unevaluated expressions that take their values from the theme.
We should ideally copy defaults from ggplot2's Geoms wherever we can, but it'd be easier when the new release is out.
| # Only register once sf is loaded | ||
| vctrs::s3_register("sf::st_as_grob", "sfc_labelled") | ||
| vctrs::s3_register("sf::st_as_grob", "sfc_textbox") |
There was a problem hiding this comment.
We didn't register these methods anywhere, but somehow it worked anyway. It is better to register them, and to do that we need to register when {sf} is loaded. We could technically avoid a dependency on {vctrs} for this, but it might have uses in the future and it is a 'free' dependency as ggplot2 also takes it as a dependency.
| point_aes <- get_geom_defaults(GeomPoint) | ||
| textpath_aes <- get_geom_defaults(GeomTextpath) | ||
| polygon_aes <- get_geom_defaults(GeomPolygon) |
There was a problem hiding this comment.
This is also to deal with the unevaluated expressions and the reason we'd require ggplot2 3.5.2
| expect_snapshot( | ||
| axis_labels$textpath$label[[9]]$glyph | ||
| ) |
There was a problem hiding this comment.
I changed these to snapshots because there is some type instability across versions. They're not run on CRAN by default, but that is fine.
|
Thanks @teunbrand. Is the new version of ggplot mainly a backend switch to S7, or are there new features? |
|
The reason why we've incremented the major version to 4.0.0 instead continuing the 3.x.x series is the S7 backend. But: there is also about a years worth of regular development in it as well, so you can read the ggplot2 news.md file to see what else is coming :) |
|
@AllanCameron Do you think we could push this to CRAN soon? Would you like any help on my end? |
|
Sorry Teun - just back from holiday and catching up. I have bumped the minor version and submitted to CRAN. |
|
Nice work, thanks for this! |
2 participants
Hi Allan,
This PR aims to bring geomtextpath up to speed with the upcoming major release of ggplot2.
I also sneaked in a small fix for #128, so that we now only throw that warnign every 5 seconds.
We plan to release ggplot2 4.0.0 in ~3 weeks so at that point it'd be great if we could send a new version with these fixes to CRAN as well.
Cheers,
Teun