From 4dedb82e46b0eca3a2e0642216e5c57f7b5926de Mon Sep 17 00:00:00 2001
From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com>
Date: Tue, 10 Dec 2024 11:10:31 +0100
Subject: [PATCH] Tweaks to `map_data()`/fortify.map()` (#6218)

* link maps as strings

* inline `fortify.map` into `map_data()`

* deprecate `fortify.map`

* add deprecation badges

* document

* throw error instead
---
 R/fortify-map.R                   | 36 ++++++++++++++++++++++++++-----
 R/fortify-spatial.R               |  3 +++
 man/fortify.map.Rd                |  2 ++
 man/fortify.sp.Rd                 |  2 ++
 man/map_data.Rd                   |  8 +++----
 tests/testthat/_snaps/geom-map.md |  9 ++++++++
 tests/testthat/test-geom-map.R    |  5 +++++
 7 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/R/fortify-map.R b/R/fortify-map.R
index d0dc76b716..e124e57e54 100644
--- a/R/fortify-map.R
+++ b/R/fortify-map.R
@@ -1,5 +1,8 @@
 #' Fortify method for map objects
 #'
+#' @description
+#' `r lifecycle::badge("deprecated")`
+#'
 #' This function turns a map into a data frame that can more easily be
 #' plotted with ggplot2.
 #'
@@ -24,6 +27,9 @@
 #'   geom_polygon(aes(group = group), colour = "white")
 #' }
 fortify.map <- function(model, data, ...) {
+  lifecycle::deprecate_warn(
+    "3.6.0", I("`fortify(<map>)`"), "map_data()"
+  )
   df <- data_frame0(
     long = model$x,
     lat = model$y,
@@ -46,10 +52,10 @@ fortify.map <- function(model, data, ...) {
 #' for plotting with ggplot2.
 #'
 #' @param map name of map provided by the \pkg{maps} package. These
-#'   include [maps::county()], [maps::france()],
-#'   [maps::italy()], [maps::nz()],
-#'   [maps::state()], [maps::usa()],
-#'   [maps::world()], [maps::world2()].
+#'   include [`"county"`][maps::county], [`"france"`][maps::france],
+#'   [`"italy"`][maps::italy], [`"nz"`][maps::nz],
+#'   [`"state"`][maps::state], [`"usa"`][maps::usa],
+#'   [`"world"`][maps::world], or [`"world2"`][maps::world2].
 #' @param region name(s) of subregion(s) to include. Defaults to `.` which
 #'   includes all subregions. See documentation for [maps::map()]
 #'   for more details.
@@ -80,7 +86,27 @@ fortify.map <- function(model, data, ...) {
 map_data <- function(map, region = ".", exact = FALSE, ...) {
   check_installed("maps", reason = "for `map_data()`.")
   map_obj <- maps::map(map, region, exact = exact, plot = FALSE, fill = TRUE, ...)
-  fortify(map_obj)
+
+  if (!inherits(map_obj, "map")) {
+    cli::cli_abort(c(
+      "{.fn maps::map} must return an object of type {.cls map}, not \\
+      {obj_type_friendly(map_obj)}.",
+      i = "Did you pass the right arguments?"
+    ))
+  }
+
+  df <- data_frame0(
+    long  = map_obj$x,
+    lat   = map_obj$y,
+    group = cumsum(is.na(map_obj$x) & is.na(map_obj$y)) + 1,
+    order = seq_along(map_obj$x),
+    .size = length(map_obj$x)
+  )
+
+  names <- lapply(strsplit(map_obj$names, "[:,]"), "[", 1:2)
+  names <- vec_rbind(!!!names, .name_repair = ~ c("region", "subregion"))
+  df[names(names)] <- vec_slice(names, df$group)
+  vec_slice(df, stats::complete.cases(df$lat, df$long))
 }
 
 #' Create a layer of map borders
diff --git a/R/fortify-spatial.R b/R/fortify-spatial.R
index 0e9f37d046..2bdcf06557 100644
--- a/R/fortify-spatial.R
+++ b/R/fortify-spatial.R
@@ -1,5 +1,8 @@
 #' Fortify method for classes from the sp package.
 #'
+#' @description
+#' `r lifecycle::badge("deprecated")`
+#'
 #' To figure out the correct variable name for region, inspect
 #' `as.data.frame(model)`.
 #'
diff --git a/man/fortify.map.Rd b/man/fortify.map.Rd
index 6ed4d9cd1b..ba1c5fbc63 100644
--- a/man/fortify.map.Rd
+++ b/man/fortify.map.Rd
@@ -14,6 +14,8 @@
 \item{...}{not used by this method}
 }
 \description{
+\ifelse{html}{\href{https://lifecycle.r-lib.org/articles/stages.html#deprecated}{\figure{lifecycle-deprecated.svg}{options: alt='[Deprecated]'}}}{\strong{[Deprecated]}}
+
 This function turns a map into a data frame that can more easily be
 plotted with ggplot2.
 }
diff --git a/man/fortify.sp.Rd b/man/fortify.sp.Rd
index af1a587f5d..0603c1ee05 100644
--- a/man/fortify.sp.Rd
+++ b/man/fortify.sp.Rd
@@ -35,6 +35,8 @@
 \item{...}{not used by this method}
 }
 \description{
+\ifelse{html}{\href{https://lifecycle.r-lib.org/articles/stages.html#deprecated}{\figure{lifecycle-deprecated.svg}{options: alt='[Deprecated]'}}}{\strong{[Deprecated]}}
+
 To figure out the correct variable name for region, inspect
 \code{as.data.frame(model)}.
 }
diff --git a/man/map_data.Rd b/man/map_data.Rd
index 5f8d2d2078..37832d29ca 100644
--- a/man/map_data.Rd
+++ b/man/map_data.Rd
@@ -8,10 +8,10 @@ map_data(map, region = ".", exact = FALSE, ...)
 }
 \arguments{
 \item{map}{name of map provided by the \pkg{maps} package. These
-include \code{\link[maps:county]{maps::county()}}, \code{\link[maps:france]{maps::france()}},
-\code{\link[maps:italy]{maps::italy()}}, \code{\link[maps:nz]{maps::nz()}},
-\code{\link[maps:state]{maps::state()}}, \code{\link[maps:usa]{maps::usa()}},
-\code{\link[maps:world]{maps::world()}}, \code{\link[maps:world2]{maps::world2()}}.}
+include \code{\link[maps:county]{"county"}}, \code{\link[maps:france]{"france"}},
+\code{\link[maps:italy]{"italy"}}, \code{\link[maps:nz]{"nz"}},
+\code{\link[maps:state]{"state"}}, \code{\link[maps:usa]{"usa"}},
+\code{\link[maps:world]{"world"}}, or \code{\link[maps:world2]{"world2"}}.}
 
 \item{region}{name(s) of subregion(s) to include. Defaults to \code{.} which
 includes all subregions. See documentation for \code{\link[maps:map]{maps::map()}}
diff --git a/tests/testthat/_snaps/geom-map.md b/tests/testthat/_snaps/geom-map.md
index 03bef91fa5..4228af232e 100644
--- a/tests/testthat/_snaps/geom-map.md
+++ b/tests/testthat/_snaps/geom-map.md
@@ -6,3 +6,12 @@
 
     `map` must have the columns `x`, `y`, and `id`.
 
+# map_data() checks it input
+
+    Code
+      map_data("world", namesonly = TRUE)
+    Condition
+      Error in `map_data()`:
+      ! `maps::map()` must return an object of type <map>, not a character vector.
+      i Did you pass the right arguments?
+
diff --git a/tests/testthat/test-geom-map.R b/tests/testthat/test-geom-map.R
index 7e02599209..6bbfb065e4 100644
--- a/tests/testthat/test-geom-map.R
+++ b/tests/testthat/test-geom-map.R
@@ -2,3 +2,8 @@ test_that("geom_map() checks its input", {
   expect_snapshot_error(geom_map(map = letters))
   expect_snapshot_error(geom_map(map = mtcars))
 })
+
+test_that("map_data() checks it input", {
+  skip_if_not_installed("maps")
+  expect_snapshot(map_data("world", namesonly = TRUE), error = TRUE)
+})