-
Notifications
You must be signed in to change notification settings - Fork 84
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
add get_* functions for plot components #111
Conversation
Codecov Report
@@ Coverage Diff @@
## master #111 +/- ##
===========================================
+ Coverage 51.64% 66.16% +14.51%
===========================================
Files 13 16 +3
Lines 697 733 +36
===========================================
+ Hits 360 485 +125
+ Misses 337 248 -89
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have commented on a few specific lines. I wouldn't say I did a comprehensive review; I just flagged things I saw. Also, please add some regression tests, in particular for component names, so we have some warning in the future should any components change name.
Great, I'll make those changes next week. Overall, is it agreeable enough that I devote some time to fleshing out the documentation? |
Yes, please go ahead. |
Please also add a bullet point to NEWS, with your github user name and PR number, as explained in the tidyverse style guide: http://style.tidyverse.org/news.html#acknowledgement |
Ok: made those changes, expanded docs, and added some tests. Not sure what's up with the Travis build. A quick look doesn't look like it's related to changes, but not sure why it would suddenly fail on this branch... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all your work. I made a few more comments. You can ignore the Travis issue, it's a separate problem that I'll have to look into.
All added. I think the Travis thing was a one-off upstream change on a distant remote repo (some r-lib upstream dependency) that already got fixed there. |
Thanks! |
Thanks, @clauswilke! |
This PR expands cowplot's functionality for extracting plot components. Closes #110.
The main addition is
get_plot_component()
, which extracts grobs from a ggplot by matching a pattern to the grob names. I've added several wrappers for axes and titles and added a couple functions for getting component names (plot_component_names()
) and quickly extracting the whole list of grobs (plot_components()
). I've also rewrittenget_legend()
andget_panel()
to useget_plot_components()
.get_legend()
works the same way, butget_panel()
can now also specify a panel in a faceted plot.I avoided the issue of pulling a list of grobs because both
+
and (by extension)ggdraw()
have trouble with them. It would probably be possible to makeggdraw()
support that, but it also doesn't really know how to put them back together. One option for pulling all facet panels is a function that puts them into a gtable, as I believe is done internally in ggplot2.This needs a little cleaning up and better documentation before it's a proper PR, but I'd thought I'd better check with you before I put that work in.
Here's a demo of the new/rewritten functions:
ggdraw(get_y_axis(p))
ggdraw(get_panel(p))
Created on 2018-10-13 by the reprex
package (v0.2.0).