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

ne_download() doens't return filename #98

Closed
eliocamp opened this issue Nov 21, 2023 · 5 comments
Closed

ne_download() doens't return filename #98

eliocamp opened this issue Nov 21, 2023 · 5 comments

Comments

@eliocamp
Copy link

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
Copy link
Contributor

PMassicotte commented Nov 21, 2023

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
Copy link

wenzmo commented Feb 15, 2024

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
Copy link
Contributor

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
Copy link

wenzmo commented Feb 16, 2024

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
Copy link

wenzmo commented Feb 16, 2024

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

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

No branches or pull requests

3 participants