Skip to content

Conversation

@ms609
Copy link
Collaborator

@ms609 ms609 commented Oct 25, 2025

This PR caches GET requests to the phylopic server.

Motivation

I'm often regenerating plots that contain phylopic silhouettes as I adjust the layout and formatting, or re-render markdown output. As it often takes a second or two to download each silhouette, calls to phylopic introduce significant delay into a workflow. As the server data is unlikely to change in the course of an R session, it seems beneficial to the user (and to the phylopic servers?) to reduce the number of GET requests

Implementation

A key is generated for each GET requests using digest, and the response is stored in a local cache environment. If the server is offline, no cache entry is created, so the call will be repeated on future attempts.

I'd considered creating a persistent cache, but this could be problematic as (i) CRAN policies are particular about how files can be created locally; (ii) longer-term caches are more likely to diverge from the server. It's not clear that there would be a sufficient benefit over session-limited caches.

Additional changes

As I went, I also added a couple of imported functions that were missing from NAMESPACE, and replaced hard-coded API references with a function (for a single point of definition).

@willgearty
Copy link
Member

Great minds think alike @ms609! I recently put together my own PR for this (#124). I'll take a look at this one when I get a chance and see if there's anything that needs to be incorporated.

@ms609
Copy link
Collaborator Author

ms609 commented Oct 27, 2025

Ah, grand, sorry I missed #124! Your solution of using an external package to manage the cache looks cleaner and more maintainable. The difference seems to be that the cache is created on disk rather than in session memory – but the pros and cons are probably immaterial for this use case.

Worth integrating the test cases from this PR to validate caching behaviour?

@willgearty willgearty added this to the 1.7.0 milestone Nov 14, 2025
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.

2 participants