Remove maptools and deprecate fortify() for sp objects

[yutannihilation]

A part of #5244

While we lean to agree on dropping these fortify() for sp objects altogether, there might be several options. At the safest, we should remove maptools, because otherwise ggplot2 might be kicked out from CRAN in the worst-case scenario. So, this pull request does

1. remove maptools
2. deprecate fortify() for sp objects in general

[teunbrand]

LGTM, but would it make sense to be a little bit more explicit why people need to migrate to {sf}?
For example: "The maptools package {is/will be} retired: please migrate to sf."

[yutannihilation]

Thanks! Sorry for replying late. I agree that this needs a bit better message, but I haven't come up with a nice one so far.

The maptools package {is/will be} retired

A bit tricky thing is that the sp package itself is not deprecated as long as the usage doesn't depend on the retiring packages (rgdal, rgeos, and maptools). It's us who want to deprecate these functions and it's not directly because of the maptools retirement, except for the defunct fortify.SpatialPolygonsDataFrame(). While I believe the r-spatial maintainers also wants to encourage the sf-ecosystem, the nuance about sp-ecosystem might be a bit different.

[teunbrand]

Right, I might've skimmed over the nuance here. In any case it was merely a suggestion, I think this PR is fine as-is.

[teunbrand]

@yutannihilation There were no reverse dependency issues with retiring maptools and it would be nice to include this PR in the hotfix for v3.4.4, as maptools retires in October. If you'd like, I'm happy to merge this in to the v3.4.4-rc and tweak the version number. If you prefer changing the target branch and changing the version number in deprecation messages yourself, that is great too.

[yutannihilation]

If you prefer changing the target branch and changing the version number in deprecation messages yourself, that is great too.

I'm fine with either, but, in my understanding, this needs to be merged into the main branch anyway, no matter whether this is merged into the RC branch or not, right?

[thomasp85]

The RC branch will be merged into main after release

[yutannihilation]

Ah, okay. I see. But, I think I cannot simply change the target branch as this contains a lot of commits from the main branch already. Isn't it easier to merge this into main first, and then cherry-pick the squashed commit?

[thomasp85]

Ah, you already merged in main. Yeah, then merging this PR and cherry picking the commit is probably the right way

[thomasp85]

hmm - can you try to change the target of the PR and see what it will change... I don't think it will bring in all the prior changes, but maybe I'm wrong

[teunbrand]

I'm not very wise in the ways of git-fu but isn't there a way to rebase a branch to 'play out' the commits over a different branch?

[yutannihilation]

Oops, I'm sorry. I actually tried to do this

I'm not very wise in the ways of git-fu but isn't there a way to rebase a branch to 'play out' the commits over a different branch?

but accidentally pushed into v3.4.4-rc directly...

https://github.com/tidyverse/ggplot2/commits/v3.4.4-rc

If this looks good to you, you can use this as it is. If not, please recreate the branch. So sorry for the trouble.

[teunbrand]

If this looks good to you, you can use this as it is.

Yeah I think it is probably fine, thanks so much for resolving this!

[yutannihilation]

@teunbrand
I see. Thanks for confirming! I'll be more careful next time... Then, I'm closing this pull request in favor of the RC branch.

@thomasp85

hmm - can you try to change the target of the PR and see what it will change... I don't think it will bring in all the prior changes, but maybe I'm wrong

I too didn't know what actually would happen, so I created a pull request on my repo, here's how it looks like:

yutannihilation#10