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

Prepare for new ggplot2 #86

Open
paleolimbot opened this issue May 29, 2021 · 5 comments
Open

Prepare for new ggplot2 #86

paleolimbot opened this issue May 29, 2021 · 5 comments

Comments

@paleolimbot
Copy link
Owner

The forthcoming release of ggplot2 will treat coordinates for non-sf geoms as WGS84 longitude/latitude. This means all geom_spatial_*() functions will stop working, as they exploit the previous behaviour (non-sf geom coords are in the CRS of coord_sf). See tidyverse/ggplot2#4494 .

The simplest workaround might be to have the geom_spatial_*() functions return list(Geom, coord_sf(..., default_crs = NULL). This would also be an opportunity to fix #60.

@clauswilke
Copy link

clauswilke commented May 29, 2021

A alternative solution, though a little more work, would be to implement an actual child geom for each geom you need, override the draw_layer() function, and modify the coord object by setting default_crs = NULL in the coord (and probably also modify is_linear() so it returns TRUE).

Though thinking this through there will likely be issues with plot limits. Not sure off the top of my head what would be the best way to address those. We'd need the ability for certain layers to be excluded from limit calculations, and I don't think there's an easy way to do this at this time.

I think the solution with list(Geom, coord_sf(..., default_crs = NULL)) is confusing because people will have plots that work and then they add coord_sf() themselves and then the plot breaks, and that will be difficult for them to figure out.

@paleolimbot
Copy link
Owner Author

Sorry for the silence, here...moving houses meant a sparse internet situation.

I agree there's no generic workaround. However, my StatSpatialIdentity could spit out a geometry column with the appropriate CRS and use GeomSf, which would make the layer CRS explicit. I'd have to make custom geometries for any non-point/line/polygon, but I it would work for current + future versions of ggplot2 without the need for a version check!

@paleolimbot
Copy link
Owner Author

paleolimbot commented Jun 1, 2021

Correcting myself...your point about scale training is a good one. The xmin/xmax/ymin/ymax is going to be all off. This is also used for plotting rasters and might not work at all if xmin/xmax/ymin/ymax have to be plate carree (pivoting extents through lon/lat will always be problematic).

@clauswilke
Copy link

However, my StatSpatialIdentity could spit out a geometry column with the appropriate CRS and use GeomSf, which would make the layer CRS explicit.

Yeah, I think this would be the best approach. It can also handle limits properly, as long as you implement appropriate limit hints:
https://github.com/tidyverse/ggplot2/blob/6d94f0d6366c86147d3409a4b6e7e046d5b7e5b0/R/stat-sf.R#L18-L24

@paleolimbot
Copy link
Owner Author

Ah yeah! That'll work perfectly for both raster and non-raster versions of this.

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

2 participants