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

Add attribution option to plotting functions #79

Merged
merged 7 commits into from
Aug 27, 2023
Merged

Conversation

willgearty
Copy link
Collaborator

@willgearty willgearty commented Aug 23, 2023

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 the name 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 if verbose is not set to TRUE.

Fixes #71.

@willgearty willgearty added this to the 1.2.0 milestone Aug 23, 2023
@willgearty willgearty self-assigned this Aug 23, 2023
@keesey
Copy link

keesey commented Aug 23, 2023

Any thoughts about utilizing the /collections endpoints on PhyloPic for attribution? You can link to pages like these: https://www.phylopic.org/collections/39f27cb5-5952-01d2-002a-0721a815ac14

There is also a way to turn those into permalinks, although I still need to document that for public usage.

@willgearty
Copy link
Collaborator Author

willgearty commented Aug 23, 2023

@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!

@willgearty
Copy link
Collaborator Author

willgearty commented Aug 23, 2023

@keesey Are collection uuids essentially permalinks? In other words, will a collections/ URL always include the same list of images?

Copy link
Collaborator

@LewisAJones LewisAJones left a 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))

R/add_phylopic.r Show resolved Hide resolved
R/add_phylopic_base.r Show resolved Hide resolved
R/add_phylopic_base.r Show resolved Hide resolved
R/add_phylopic_base.r Outdated Show resolved Hide resolved
R/geom_phylopic.R Outdated Show resolved Hide resolved
R/geom_phylopic.R Outdated Show resolved Hide resolved
R/add_phylopic_base.r Outdated Show resolved Hide resolved
R/add_phylopic.r Outdated Show resolved Hide resolved
Co-authored-by: Lewis A. Jones <41071747+LewisAJones@users.noreply.github.com>
@willgearty
Copy link
Collaborator Author

willgearty commented Aug 25, 2023

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?

@LewisAJones
Copy link
Collaborator

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).

@willgearty
Copy link
Collaborator Author

willgearty commented Aug 25, 2023

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...

@LewisAJones
Copy link
Collaborator

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!

@willgearty
Copy link
Collaborator Author

I decided to go ahead and make the change anyways. Let me know what you think @LewisAJones!

@keesey
Copy link

keesey commented Aug 25, 2023

@keesey Are collection uuids essentially permalinks? In other words, will a collections/ URL always include the same list of images?

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.)

Copy link
Collaborator

@LewisAJones LewisAJones left a 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.

@willgearty willgearty merged commit 74b56df into main Aug 27, 2023
8 checks passed
@willgearty willgearty deleted the verbose-geom branch August 27, 2023 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

verbose geom_phylopic
3 participants