16EAGLE / basemaps

A lightweight package for accessing basemaps from open sources in R 🗺️
https://jakob.schwalb-willmann.de/basemaps
GNU General Public License v3.0
57 stars 15 forks source link

Issues related to caching #11

Closed benscarlson closed 1 year ago

benscarlson commented 2 years ago

I believe I've stumbled upon a couple of possibly inter-related issues with the caching code.

First, if I set something like the following:

basemap_ggplot(ext=ext,map_service='esri',map_type='world_imagery',verbose=TRUE, map_dir=file.path(.wd,'mymaps'))

png files are correctly placed in the mymaps folder. However, a file like mymapsbasemap_20220829115215.tif is placed in my working directory. Is this also supposed to be placed in the mymaps directory? If so, I think the culprit is here: https://github.com/16EAGLE/basemaps/blob/e86cd622c4027f4467b216e392746da05f92cad2/R/internal.R#L150

Perhaps you meant do so something like file.path(map_dir, ...) instead of paste0(map_dir,...) ?

Secondly, the basemap_* function throws an error message if you change the cache within the session. If I executed the code above, then execute (changing mymaps to mytiles):

basemap_ggplot(ext=ext,map_service='esri',map_type='world_imagery',verbose=TRUE, map_dir=file.path(.wd,'mytiles'))

I receive the error message

Error in cached[[which(cached.match)]]$file_comp : $ operator is invalid for atomic vectors

It seems to come from here: https://github.com/16EAGLE/basemaps/blob/e86cd622c4027f4467b216e392746da05f92cad2/R/internal.R#L148

16EAGLE commented 2 years ago

Perhaps you meant do so something like file.path(map_dir, ...) instead of paste0(map_dir,...) ?

Thanks, that is clearly a bug – fixed this with 545fc49.

Secondly, the basemap_* function throws an error message if you change the cache within the session. If I executed the code above, then execute (changing mymaps to mytiles):

I could not exactly reproduce this, however, changing map_dir during a session definitely caused issues. When changing map_dir, all maps that had been previously cached in the current session were still considered even if they had been saved to a different directory, which was not intended. Cached maps that are not stored in the current map directory are now disregarded, so that only maps stored in map_dir are considered to be recycled (21daaa5).

Could you test whether the issue is solved with the most recent commits?

benscarlson commented 1 year ago

Both fixes worked!

library(basemaps)
packageDescription('basemaps')$Version #0.0.4
data(ext)

Validate that fix 545fc49 places the .tif file in map_dir

mp <- basemap_ggplot(ext=ext,map_service='esri',map_type='world_imagery',
  verbose=TRUE, map_dir='mymaps')

Validate that fix 21daaa5 does not throw an issue and does not attempt to load from cache if map_dir is changed

mp2 <- basemap_ggplot(ext=ext,map_service='esri',map_type='world_imagery',
  verbose=TRUE, map_dir='mytiles')