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

Resolve name conflicts: as_grob() #131

Closed
clauswilke opened this issue Dec 1, 2019 · 0 comments
Closed

Resolve name conflicts: as_grob() #131

clauswilke opened this issue Dec 1, 2019 · 0 comments

Comments

@clauswilke
Copy link

@clauswilke clauswilke commented Dec 1, 2019

Both cowplot and patchwork define generics as_grob(). However, cowplot exports it and patchwork does not. It seems like the easiest solution for now, to avoid any immediate problems, would be to rename the internal patchwork function. If you agree, I'm happy to prepare a PR.

Going forward, we should have a discussion of what this type of function should do and where it should live. The main difference I see is that your function doesn't open a background device for as_grob.ggplot() whereas mine does. I think both approaches are needed. When you're calling as_grob.ggplot() during the final output phase, for example as part of a ggsave() call, then opening a background device is not needed (and is actually harmful). But if you're calling as_grob.ggplot() before final output is generated, e.g. because you're aligning plots that will be output later, then not having an open background device can cause weird bugs in some settings, at least it has done so for me in the past (wilkelab/cowplot#24 wilkelab/cowplot#51 wilkelab/cowplot#73 wilkelab/cowplot#82).

@thomasp85 thomasp85 closed this in 3b5461d Dec 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.