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

Clarify the current status of fortify() #3816

Closed
3 tasks done
yutannihilation opened this issue Feb 11, 2020 · 6 comments · Fixed by #6218
Closed
3 tasks done

Clarify the current status of fortify() #3816

yutannihilation opened this issue Feb 11, 2020 · 6 comments · Fixed by #6218
Labels
breaking change ☠️ API change likely to affect existing code layers 📈

Comments

@yutannihilation
Copy link
Member

yutannihilation commented Feb 11, 2020

(Originally commented at #3802 (comment))

In my understanding, there are 2 purposes of fortify().

  1. User-facing utility to convert an object to data.frame, which otherwise the users don't know how to.
  2. API for developers that let the custom objects handled nicely in the internal of ggplot2 by tweaking the object at the very first stage.

For purpose 1, ggplot2 now provides 2 types of methods, both of which are becoming less important:

  • Convert model objects, which is superseded broom package. In addition, ggfortify covers much wider range of objects.
  • Convert geospatial objects (of sp and maps), which is being superseded by sf package and geom_sf().

So, we can and should deprecate (or at least discourage) this type of usages.

On the other hand, for purpose 2, I think fortify() still plays a somehow important role. For example, fortify.tbl() ensures tbl is collect()ed.

ggplot2/R/fortify.r

Lines 19 to 24 in 0137a4d

fortify.tbl <- function(model, data, ...) {
if (!requireNamespace("dplyr", quietly = TRUE)) {
abort("dplyr must be installed to work with tbl objects")
}
dplyr::collect(model)
}

Suggestions

I suggest

  • Deprecate fortify.lm(), fortify.glht(), fortify.confint.glht(), fortify.summary.glht() and fortify.cld(). Replace examples using broom::augment() and broom::tidy().
  • Deprecate fortify.<sp-object> and fortify.map() after ensuring geom_sf() supersedes geom_map().
  • Merge the docs of fortify() to autoplot() and autolayer(), and explain how developers of some custom objects can implements these methods so that the users can use ggplot2 to visualize the objects painlessly (Mark geom_map() as superseded #3721).

Note that this issue is not very high priority as maintaining fortify() doesn't seem a heavy burden, at least at the moment. I filed this issue just because it seems we need some place to discuss to avoid confusion. Anyway, I believe it's a right move to encourage users to use broom and sf.

@hadley hadley added layers 📈 breaking change ☠️ API change likely to affect existing code labels Mar 14, 2022
@hadley
Copy link
Member

hadley commented Mar 14, 2022

This sounds like a good proposal to me, but it'll need to wait until a major/minor release.

@teunbrand
Copy link
Collaborator

I'm considering the docs bullet adressed by #5745

@yutannihilation
Copy link
Member Author

  • Deprecate fortify.<sp-object> and fortify.map()

fortify.<sp-object> is already deprecated by #5327, but it seems fortify.map() is not yet.

@teunbrand
Copy link
Collaborator

We can consider putting up map_data() + fortify.map() for #5469. In #3721 it was discussed to mark geom_map() as superseded, but I think this plan was ultimately abandoned in favour of making geom_map() compatible with the {sf} infrastructure.

@yutannihilation
Copy link
Member Author

In #3721 it was discussed to mark geom_map() as superseded, but I think this plan was ultimately abandoned

Thanks for the summary. The nuance might vary a bit, but I agree the conclusion was something like this.

Yet, I think deprecating fortify.map() is possible. The examples on ?geom_map uses the data generated by map_data(), which uses fortify.map() internally.

ggplot2/R/fortify-map.R

Lines 80 to 84 in 3d67907

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)
}

I have almost zero knowledge about map_geom() so I might be wrong, but probably it's rare for users to use fortify.map() by themselves? If so, it's not so risky to deprecate it. map_data() can be tweaked to inline the code of fortify.map(). (I'm also hoping to find some sf alternative for base maps instead of {mapdata}.)

@yutannihilation
Copy link
Member Author

It's great to see this issue is closed at last. Thanks @teunbrand for your hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change ☠️ API change likely to affect existing code layers 📈
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants