-
Notifications
You must be signed in to change notification settings - Fork 9
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 attribution option to plotting functions #79
Conversation
Any thoughts about utilizing the There is also a way to turn those into permalinks, although I still need to document that for public usage. |
@keesey Ah, I had always thought you needed to create collections in the web portal, but I see now you can generate them with the API. I'll look into incorporating that! |
@keesey Are collection uuids essentially permalinks? In other words, will a collections/ URL always include the same list of images? |
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.
Hi @willgearty, this is looking pretty good and almost there. The main issue I see is that with geom_phylopic
the warning message and attribution information is called for each unique name, see example:
library(ggplot2)
df <- data.frame(x = c(2, 4), y = c(10, 20), name = c("cat", "walrus"))
# Multiple warning messages
ggplot(df) +
geom_phylopic(aes(x = x, y = y, name = name),
verbose = FALSE, color = "purple", size = 10) +
facet_wrap(~name) +
coord_cartesian(xlim = c(1,6), ylim = c(5, 30))
# Multiple attribution messages (should just be one).
ggplot(df) +
geom_phylopic(aes(x = x, y = y, name = name),
verbose = TRUE, color = "purple", size = 10) +
facet_wrap(~name) +
coord_cartesian(xlim = c(1,6), ylim = c(5, 30))
Co-authored-by: Lewis A. Jones <41071747+LewisAJones@users.noreply.github.com>
I think it's once per facet/panel for geom_phylopic? I figured this was fine because often you list separate captions for each facet/panel of a figure anyways? |
Maybe? I can just imagine a situation where you have many facets and you flood the console (like our LBG paper for example). |
Hmm...that might require a complete refactoring of GeomPhylopic, then. We currently process the names/uuids/imgs on a per-panel basis (this is pretty standard for geoms), so we would need to move all of this processing to a different step of the geom construction process. I'll look into it... |
Sounds like an update for a later version. Happy with moving forward as is! |
I decided to go ahead and make the change anyways. Let me know what you think @LewisAJones! |
It will always include the same list of UUIDs, but it's always possible that images will be removed or details changed. (Although both of those are quite infrequent.) Collections can be turned into actual permalinks, which will never change, but I haven't moved that to the general API yet. (Not sure when I'll have bandwidth.) |
c27fb8d
to
578e3c7
Compare
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.
All seems good to me now @willgearty, thanks for addressing my previous concern in this PR! I hope it wasn't too much extra work.
This adds the
verbose
argument to our three plotting functions (geom_phylopic, add_phylopic, and add_phylopic_base) that outputs the get_attribution text for the silhouettes used. If thename
aesthetic/argument is used in these functions (implying the user doesn't have a list of uuids or image objects in their environment), a warning is printed ifverbose
is not set toTRUE
.Fixes #71.