paleolimbot / ggspatial

Enhancing spatial visualization in ggplot2
https://paleolimbot.github.io/ggspatial
368 stars 34 forks source link

Prepare for new ggplot2 #86

Open paleolimbot opened 3 years ago

paleolimbot commented 3 years ago

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 https://github.com/tidyverse/ggplot2/issues/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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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