Skip to content

Enable geom_sf to automatically determine the legend type #3646

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.

Sign up for GitHub

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

Merged
merged 18 commits into from
Dec 6, 2019
Merged

Enable geom_sf to automatically determine the legend type #3646

Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
2fd47b8
Merge pull request #2 from tidyverse/master
microly Oct 24, 2019
40bdc73
Merge pull request #3 from tidyverse/master
microly Nov 18, 2019
346a8f0
Merge pull request #4 from tidyverse/master
microly Nov 29, 2019
f8a85ef
automatically determine the legend type
microly Nov 29, 2019
68a3599
drop "%>%"
microly Dec 1, 2019
5927cc9
add "sf::"
microly Dec 1, 2019
c6f6565
use "isTRUE()"
microly Dec 2, 2019
95e29fa
kind instructions from @clauswilke
microly Dec 2, 2019
465f623
I think it works! Need to clean code comments.
microly Dec 3, 2019
e1d3de5
I think this is the final edition -_-!
microly Dec 5, 2019
60ed8db
add test
microly Dec 5, 2019
34dee47
add more tests to confirm that automatic choice can be overridden man…
microly Dec 5, 2019
4313b5e
handle layer.r
microly Dec 6, 2019
a8e1e3a
Merge pull request #5 from tidyverse/master
microly Dec 6, 2019
78e5fde
handle NEWS.md
microly Dec 6, 2019
6dfb21f
Merge branch 'layer-sf_patch1_auto-legend-type' of https://github.com…
microly Dec 6, 2019
c8e633a
kind suggestions from @yutannihilation, thanks! You are good!
microly Dec 6, 2019
841c3fb
I am so sorry...
microly Dec 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions R/layer-sf.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ LayerSf <- ggproto("LayerSf", Layer,
self$mapping$geometry <- as.name(geometry_col)
}
}

# automatically determine the legend type
if (is.na(self$show.legend) || isTRUE(self$show.legend)) {
if (is_sf(data)) {
if (sf_geometry_type(data) %in% c("POINT", "MULTIPOINT"))
self$geom_params$legend <- "point"
else if (sf_geometry_type(data) %in% c("LINESTRING", "MULTILINESTRING",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're calling sf_geometry_type() twice. Better to call it once, assign the result to a temp variable, and use that in the if statements.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry that I can not express all my thanks in English! I feel that I am a young pupil and you are my teacher who teaches me hand by hand. Thanks!

"CIRCULARSTRING", "COMPOUNDCURVE",
"CURVE", "MULTICURVE"))
self$geom_params$legend <- "line"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably you can use sf_types instead of a raw list of sf types.

ggplot2/R/geom-sf.R

Lines 300 to 306 in 6424808

sf_types <- c(GEOMETRY = "other", POINT = "point", LINESTRING = "line",
POLYGON = "other", MULTIPOINT = "point", MULTILINESTRING = "line",
MULTIPOLYGON = "other", GEOMETRYCOLLECTION = "collection",
CIRCULARSTRING = "line", COMPOUNDCURVE = "other", CURVEPOLYGON = "other",
MULTICURVE = "other", MULTISURFACE = "other", CURVE = "other",
SURFACE = "other", POLYHEDRALSURFACE = "other", TIN = "other",
TRIANGLE = "other")

}
}
data
}
)
Expand Down Expand Up @@ -62,3 +74,9 @@ is_sf <- function(data) {
#' @export
scale_type.sfc <- function(x) "identity"

# helper function to determine the geometry type of sf object
sf_geometry_type <- function(sf) {
geometry_type <- unique(as.vector(sf::st_geometry_type(sf)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why as.vector()? Why not as.character()?

if (length(geometry_type) != 1) geometry_type <- "blend"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't like "blend". It is introduced here without documentation and not used anywhere else. I'd be fine with "other", since that is used elsewhere in the code.

geometry_type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this function should return sf_types[geometry_type] here, to simplify the code further up. I would also propose renaming it, since sf_geometry_type() sounds so similar to sf::st_geometry_type() but does somewhat different things. I recommend the name detect_geometry_type().

}