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

geom_sf does not pass parameters through to layer #2826

Closed
alistaire47 opened this issue Aug 12, 2018 · 4 comments
Closed

geom_sf does not pass parameters through to layer #2826

alistaire47 opened this issue Aug 12, 2018 · 4 comments

Comments

@alistaire47
Copy link
Contributor

@alistaire47 alistaire47 commented Aug 12, 2018

Due to this SO question, I discovered that while geom_sf does pass aesthetics through to the appropriate layer (here to plot linestrings as paths), e.g. size, it doesn't pass parameters like lineend, linemitre, linejoin, and arrow, which are parameters of geom_path, not aesthetics like size or linetype. In this case, this makes getting rid of the gaps in this plot harder:

# Reprex from SO question linked above
library(sf)
#> Linking to GEOS 3.6.1, GDAL 2.1.3, proj.4 4.9.3
library(osmdata)
#> Data (c) OpenStreetMap contributors, ODbL 1.0. http://www.openstreetmap.org/copyright
library(ggplot2)

# define bounding box for osm data
my_bbox <- matrix(
    c(-0.2605616, -0.2605616,
      -0.2004485, -0.2004485,
      -0.2605616, 51.4689943,
      51.4288980, 51.4288980,
      51.4689943, 51.4689943),
    ncol = 2
)
bbox_sf <- st_geometry(st_polygon(x = list(my_bbox)))
st_crs(bbox_sf) <- 4326

# get osm road data for bounding box
osm_roads_secondary_sf <- 
    opq(bbox = st_bbox(bbox_sf)) %>%
    add_osm_feature(key = 'highway', value = 'secondary') %>%
    osmdata_sf()

ggplot(osm_roads_secondary_sf$osm_lines) + 
    geom_sf(size = 4, lineend = "round")
#> Warning: Ignoring unknown parameters: lineend

According to ?geom_sf, these parameters get passed through ... to the paired geom/stat or layer, though that function doesn't have parameters for everything or dots—I'm assuming they're passed to its params parameter. The problem here seems to be that the paired stat here is stat_sf, which does not have the specified parameters and is not passing them along further.

Ultimately this behavior is a product of the fact that geom_sf dispatches to so many...geoms? As a sort of meta-geom it's more strongly opinionated than the rest of the API, making it hard to, say, use sf points as locations for geom_text without pulling out the coordinates. Guidance in the docs on what sf geometry type dispatches to what geom (?) (and whether there's a way to override that dispatching) and thus what parameters can be passed would be welcome.

@clauswilke
Copy link
Member

@clauswilke clauswilke commented Aug 13, 2018

The lineend setting is hardcoded here:

ggplot2/R/sf.R

Line 210 in 5f868c5

lineend = "butt"

To make it configurable, it needs to be made an argument of the draw_panel() function:

ggplot2/R/sf.R

Line 154 in 5f868c5

draw_panel = function(data, panel_params, coord, legend = NULL) {

and then passed through to sf_grob():

ggplot2/R/sf.R

Line 189 in 5f868c5

sf_grob <- function(row) {

(and maybe the same should be done for the linejoin and linemitre parameters).

The draw_panel() function from GeomPath can serve as an example:

ggplot2/R/geom-path.r

Lines 143 to 145 in 5f868c5

draw_panel = function(data, panel_params, coord, arrow = NULL,
lineend = "butt", linejoin = "round", linemitre = 10,
na.rm = FALSE) {

Do you want to prepare a pull request?

@clauswilke
Copy link
Member

@clauswilke clauswilke commented Aug 13, 2018

And if you want to document these parameters and their default settings, you should add them to the constructor functiongeom_sf() as well, as can be seen here for geom_path():

ggplot2/R/geom-path.r

Lines 90 to 117 in 5f868c5

geom_path <- function(mapping = NULL, data = NULL,
stat = "identity", position = "identity",
...,
lineend = "butt",
linejoin = "round",
linemitre = 10,
arrow = NULL,
na.rm = FALSE,
show.legend = NA,
inherit.aes = TRUE) {
layer(
data = data,
mapping = mapping,
stat = stat,
geom = GeomPath,
position = position,
show.legend = show.legend,
inherit.aes = inherit.aes,
params = list(
lineend = lineend,
linejoin = linejoin,
linemitre = linemitre,
arrow = arrow,
na.rm = na.rm,
...
)
)
}

@alistaire47
Copy link
Contributor Author

@alistaire47 alistaire47 commented Aug 14, 2018

Ok! It's a busy week, but you've left a good path here, so with a little luck I should be able to get this together relatively quickly.

clauswilke added a commit that referenced this issue Aug 23, 2018
Closes #2826

* fixed discrete_scale documentation

Updated `discrete_scale` documentation to match functionality and
`continuous_scale` (@alistaire47, $2052).

* Cleaned up documentation inheritance

* add line parameters to geom_sf #2826

* add line parameters to geom_sf (#2826)

* refactor to add line parameters to sf_grob

* updated NEWS.md for #2826

* Added Oxford comma
@lock
Copy link

@lock lock bot commented Feb 19, 2019

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 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants