h-a-graham / rayvista

An R plugin for {rayshader} to view a 3D vista anywhere on earth.
GNU General Public License v3.0
107 stars 5 forks source link

bounds object not recognised #28

Closed joelfiddes closed 3 years ago

joelfiddes commented 3 years ago

Hi there, this is a really geat looking package and have just been playing with a fresh install, however, I am getting an error with the crop function:

Seems like the bounds object generated here in download_elevation.R: bounds <- sf::st_bbox(bounds_sf)

is not recognised as the crop function then throws: Error in .local(x, y, ...) : Cannot get an Extent object from argument y

Fine with all other elevation datasets args - just applies to "aws" where you have the patch from commit: https://github.com/h-a-graham/rayvista/commit/9ae923c99d67265d93c8c5c5afad0f2c4dde480b

Thanks!

R version 3.6.3

h-a-graham commented 3 years ago

Hi, I'm not able to reproduce this on my machine. Can you please share your sessionInfo()?

So we have a reproducible example to work with - Does this example produce the same error for you?

library(rayvista)
.lat <- 57.219566
.long <- -6.092690
cuillins <- plot_3d_vista(lat = .lat, long = .long, phi=30, 
                          elevation_src = 'aws', outlier_filter =0.001)

FYI I am also (slowly) working on a different version of this package which I've not shared yet which should hopefully overcome many of these types of problems which I suspect may result from differing/old versions some of the rspatial packages.

Cheers!

h-a-graham commented 3 years ago

Could you please try the following?

raster::extent(sf::st_bbox(c(xmin=1,ymin=1,xmax=2, ymax=2)))

I am assuming that the installed version of raster is outdated and doesn't support creating a raster Extent object from an sf bbox. I am hoping that the above throws an error which would make sense if the raster version you are using is < 3.3-15. creating an extent from st_bbox was added here: https://github.com/rspatial/raster/commit/caf4148d87cd0ec97eef666a36c525d91df37895

joelfiddes commented 3 years ago

Hi Graham thanks for looking into it and looks like you've solved it. Your test does indeed throw an error and I have raster_3.0-7 sp_1.4-4 running. Apologies, I should have thought of this!

h-a-graham commented 3 years ago

No problem at all! It's very helpful actually, I'll add the version number as a minimum dependency. Thanks very much.

h-a-graham commented 3 years ago

OKay so I've added the minimum raster dep here:m https://github.com/h-a-graham/rayvista/pull/29

I'll close this issue for now but please reopen if the problem persists.

Many thanks!