Tweaks to map_data()/fortify.map()`
#6218
Conversation
R/fortify-map.R
Outdated
| if (!inherits(map_obj, "map")) { | ||
| return(fortify(map_obj)) | ||
| } |
There was a problem hiding this comment.
Do you know when maps::map() returns a non-map object? Maybe we should just ignore, or raise an error rather than forwarding to fortify()?
map <- c("county", "france", "italy", "nz", "state", "usa", "world", "world2")
vapply(
map,
\(x) class(maps::map(x, region = ".", exact = FALSE, plot = FALSE, fill = TRUE)),
FUN.VALUE = character(1L)
)
#> county france italy nz state usa world world2
#> "map" "map" "map" "map" "map" "map" "map" "map"Created on 2024-12-10 with reprex v2.1.1
There was a problem hiding this comment.
If a user passes the namesonly = TRUE option, the maps::map() function does not return a non-map object.
Currently we throw the default error message, which will continue to be the case with this PR.
x <- maps::map(namesonly = TRUE, plot = FALSE)
str(x)
#> chr [1:1627] "Aruba" "Afghanistan" "Angola" "Angola:Cabinda" "Anguilla" ...
ggplot2::map_data("world", namesonly = TRUE)
#> Error in `fortify()`:
#> ! `data` must be a <data.frame>, or an object coercible by `fortify()`,
#> or a valid <data.frame>-like object coercible by `as.data.frame()`.
#> Caused by error in `.prevalidate_data_frame_like_object()`:
#> ! `dim(data)` must return an <integer> of length 2.Created on 2024-12-10 with reprex v2.1.1
Happy to throw a more custom error message if you think that is more appropriate!
There was a problem hiding this comment.
Oh, I didn't know this, thanks.
Happy to throw a more custom error message if you think that is more appropriate!
IMHO, throwing an error is preferable because we can avoid unnecessary fortify().
How about something like `maps::map()` didn't return a map object. Did you pass some unusual options to `map_data()`? ? I think this is a corner case, so I don't think it needs to be very user-friendly.
There was a problem hiding this comment.
Alright, now throwing the following error:
devtools::load_all("~/packages/ggplot2/")
#> ℹ Loading ggplot2
map_data("world", namesonly = TRUE)
#> Error in `map_data()`:
#> ! `maps::map()` must return an object of type <map>, not a character
#> vector.
#> ℹ Did you pass the right arguments?Created on 2024-12-10 with reprex v2.1.1
There was a problem hiding this comment.
The error looks good to me!
|
Thanks for the review! |
This PR aims to fix #3816, close #5469 and fix #3717.
With regards to #3816, this PR checks off the last bullet that
fortify.map()is deprecated.WIth regards to #5469, this PR makes the issue obsolete, since we have deprecated all the functions we were considering putting up for adoption.
With regards to #3717 this PR fixes the mislinked
map_data(map)argument.