ropensci / rnaturalearth

An R package to hold and facilitate interaction with natural earth map data :earth_africa:
http://ropensci.github.io/rnaturalearth/
Other
214 stars 24 forks source link

`ne_download()` doens't return filename #98

Closed eliocamp closed 4 months ago

eliocamp commented 7 months ago

ne_download() returns the name of the file without the extension. Reprex:

folder <- tempdir()
file <- rnaturalearth::ne_download(category = "raster", type = "HYP_50M_SR_W", 
                                   scale = 50, load = FALSE, destdir = folder)
file.exists(file.path(folder, file))
#> [1] FALSE

file.exists(file.path(folder, paste0(file, ".tif")))
#> [1] TRUE

Created on 2023-11-21 with reprex v2.0.2

Changing this would be a breaking change, though.

If you think it's worth changing, I would suggest returning the full path to the file so the user could do

ne_download(...) |> 
   sf_read()  # Or other read function
PMassicotte commented 7 months ago

Maybe it should simply return the full name of the file.

https://github.com/ropensci/rnaturalearth/blob/d19c1875edd8f145551ee697b1451d017e78b048/R/ne_download.R#L149C6-L149C6

Also need to find out the proper file extension:

ext <- switch(category,
  "raster" = ".tif",
  ".shp"
)

This is assuming that all vector data is returned as shapefiles (which I think is the case).

wenzmo commented 4 months ago

I think the problem is in the unzip function. When it will unzipped a new folder with the same name will be created but the path is created without. Can you change this?

From ne_download():

Lines 25 to 30:

utils::unzip(zip_file,` exdir = destdir)
  if (load && category == "raster") {
    rst <- terra::rast(file.path(destdir, paste0(file_name, 
      ".tif")))
    return(rst)
  }

Should be:

utils::unzip(zip_file, exdir = destdir)
  if (load && category == "raster") {
    rst <- terra::rast(file.path(destdir, paste0(file_name, "/", file_name,
      ".tif")))
    return(rst)
  }

At least this should work even if its ugly

PMassicotte commented 4 months ago

I am not sure to understand. You have an issue when loading a raster? For me it works. The issue is the last line of the function which return the filename when the data is not loaded.

wenzmo commented 4 months ago

Ok, I am sorry. That was not clear enough. The code I used was

ne_download(type = "MSR_50M", category = "raster",
                      scale = 50, returnclass = "sf", load = TRUE)

Then I got this error:

Error: [rast] file does not exist: .../AppData/Local/Temp/RtmpAZAqZf/MSR_50M.tif In addition: Warning message: .../AppData/Local/Temp/RtmpAZAqZf/MSR_50M.tif: No such file or directory (GDAL error 4) (.. the dots represent the real path and don't want to show it)

So I checked in the temp location if the file is existing. And it does, but whithin a folder named MSR_50M. That was my recommondation, to add the file name as a folder name as well.

I checked the ne_load() function and this has this file_tif <- file.path(destdir, file_name, paste0(file_name, ".tif")) inside as I recommended to add in the ne_download() as well.

wenzmo commented 4 months ago

Or in short, the argument load = True in the ne_download() does not work