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

pathGrob usage failure #3312

Closed
paleolimbot opened this issue May 8, 2019 · 5 comments
Closed

pathGrob usage failure #3312

paleolimbot opened this issue May 8, 2019 · 5 comments
Milestone

Comments

@paleolimbot
Copy link
Member

This is a revdep issue (#3303) in which there is an installation warning about pathGrob() not having a pathId argument:

possible error in 'pathGrob(munched$x, munched$y, ': unused argument (pathId = munched$group)

This affects ggalt, ggforce, ggpol, ggpolypath, ggsolvencyii, and ggspatial. I'm guessing this has to do with #3128 (this is why ggspatial is using pathGrob...to put holes in polygons), but I don't know why.

@paleolimbot paleolimbot added this to the ggplot2 3.2.0 milestone May 8, 2019
@thomasp85
Copy link
Member

I’ll take this once I get on top — this is my code all the way down

@clauswilke clauswilke mentioned this issue May 10, 2019
17 tasks
@thomasp85
Copy link
Member

ok, so this is the same story as I had with sf, back when I added the new pathGrob capabilities... for weird reasons ggplot2 does not itself trigger this build warning, but packages subclassing polygonGrob will...

The fix is to mask pathGrob so it doesn't get called directly (this is super stupid as the current code is correct, but the static analyser is throwing a fit)... copying the sf approach should fix it

https://github.com/r-spatial/sf/blob/b860ad64ce3f7d1f5fc787048237fb952b226325/R/init.R#L31-L39

I hope someone else can take this up as my head is still not happy about prolonged computer use (reading is better than actively working)

@paleolimbot
Copy link
Member Author

Just clarifying...this is something we don't have to fix in ggplot2, we just have to notify the downstream package developers? If so, I'd be happy to do this.

@thomasp85
Copy link
Member

We should do this in ggplot2. ggforce doesn’t call pathGrob() directly, only through inheritance of GeomPolygon so it is something we should fix here by masking grid::pathGrob as I did for sf

paleolimbot added a commit to paleolimbot/ggplot2 that referenced this issue May 15, 2019
…class GeomPolygon from throwing an install warning. Fixes tidyverse#3312.
@lock
Copy link

lock bot commented Nov 11, 2019

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Nov 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants