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

Encapsulate sf_grob() in GeomSf$draw_panel() #5904

Merged
merged 4 commits into from
Jul 11, 2024

Conversation

teunbrand
Copy link
Collaborator

This PR does not really solve an open issue, but does make GeomSf more consistent with other geoms.

Briefly, sf_grob() was a wrapper for sf::st_as_grob() that did two things that are usually handled by Geom classes.
It removed missing data, which is usually handled by Geom$handle_na(), and wrangles the graphical parameters for a grob, which is usually done in Geom$draw_panel().
This PR divides these two responsibilities to the appropriate places in the class.

Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Merge branch 'main' into sf_remove_missing

# Conflicts:
#	R/geom-sf.R
@teunbrand teunbrand merged commit 744e021 into tidyverse:main Jul 11, 2024
11 checks passed
@teunbrand teunbrand deleted the sf_remove_missing branch July 11, 2024 17:44
@arcresu
Copy link

arcresu commented Oct 10, 2024

@teunbrand after this refactor lty = linetype is no longer passed to gpar for GeomSf. Was this intentional?

@teunbrand
Copy link
Collaborator Author

No that is not intentional, thanks for reporting on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants