spatial-data-science-and-geo-ai-lab / greenexp_r Goto Github PK

this should be unreachable due to the conditional tests in lines 65-76.

Note that constructing pathnames with string concatenation like this can cause problems on systems that don't use forward slash as directory separator. Although these days that is mostly just Windows & probably R handles that for you. But just be aware.

There's a 'proper' way to do it:
https://stat.ethz.ch/R-manual/R-devel/library/base/html/file.path.html

I picked this example for the bug report, but like the other reports, there are multiple instances of this problem, I will only report one though.

Note that year here is a string (character) not a number so the <, > operators are probably doing lexicographical comparison, not numerical. If someone provides a string like '2020a' it would pass your check even though that is a bad input. The initial thought is just to check directly if the year is '2020' or '2021' explicitly, no need to use inequalities.

More generally, this is something that will probably change in the future and for production purposes it's probably best not to hardcode these years into the code. Can valid years be detected from the planetary computer API?

Access tokens should be kept secret and not committed to a public repo. Please go to your mapillary API key management webpage, delete this one and make a new one. Then delete this line and arrange to have the access key loaded somehow from a resource not in the repo. Either that can be a file that is explicitly NOT kept in the repo (e.g. 'api.key' or with a filename provided as an option), on the command line, from the environment variables, or make it possible to do any of the above, to give users the maximum convenience.

If you go with something like 'api.key' it is handy to add that name (or a wildcard like *.key) to the .gitignore file.

Build-generated files like those ending with .so, .o and .dll should not be in the repository, and should be added to .gitignore to prevent them from being accidentally added.

This seems to assume that there is a 1:1 inverse proportional correlation between number of 'cores' used for processing and running time. But that is not something you can assume, it would only be the case if the task was perfectly parallelisable and there were no other issues (e.g. contention on the bus).

change the error message to be 'speed must be a positive number'

Note that there is a thumb_original_url feature that gives you the original image back, this seems like it would be a bit nicer to have?

It would be better to extract the year from the date_time and then compare it to the year variable, rather than crudely construct a timestamp for midnight of Jan 1st of that year.

I am facing different types of error a problem with installing this software.

It is probably faster to use sort() and then rle() (then dropping the count information), if you are really concerned about speed here. But it's also probably micro-optimisation that is unnecessary. Just noting that unique() followed by sort() is performing redundant computation, as both run cross-comparisons across the whole list. If you need sorted 'unique-ified' output then sort first and remove duplicates using something like rle() which should be linear in running time rather than quadratic (and therefore less intensive than O(n log n) for sort).

Hello. The code in examples runs error on my computer, it reports 'no slot of name "ptr" for this object of class "SpatRaster"'

#For this example, we first need to download the DSM, DTM and Green space layer from the following url (where we already uploaded the data)

#create a temp diractory to get the zip file
temp <- tempfile(fileext = "zip")
download.file("https://a35b3ff7-2e06-4a4f-a669-8681322e59a7.usrfiles.com/archives/a35b3f_c9ec6cb349d043f4b3b055d6dc984ed3.zip", temp)

#extract the zip file
out <- unzip(temp, exdir = tempdir())

#load the DSM, DTM, and GreenSpace (GS)
DSM <- terra::rast(out[1])
DTM <- terra::rast(out[2])
GS <-  terra::rast(out[3])

#let us fix an obersver location 
obersver_mp <- sf::st_sf(sfheaders::sf_point(c(4.882752, 52.358029)), crs = st_crs(4326))

#make sure the oberver location has same projection as the DSM/DTM/GS file. They all need to be in same projection
obersver_mp <- sf::st_transform(obersver_mp, sf::st_crs(DSM))

singleview <- GreenExp::viewshed(observer = obersver_mp, dsm_rast = DSM, dtm_rast = DTM,
                          max_distance = 200, observer_height = 1.7, plot = TRUE)

Results:

trying URL 'https://a35b3ff7-2e06-4a4f-a669-8681322e59a7.usrfiles.com/archives/a35b3f_c9ec6cb349d043f4b3b055d6dc984ed3.zip'
Content type 'application/zip' length 7365204 bytes (7.0 MB)
downloaded 7.0 MB

Error in GreenExp::viewshed(observer = obersver_mp, dsm_rast = DSM, dtm_rast = DTM,  :
  no slot of name "ptr" for this object of class "SpatRaster"

For recent terra package, it shows:

r$> getSlots("SpatRaster")
        pnt
"C++Object"

I'm not sure if the slot in terra SpatRaster was renamed from ptr to pnt. Thank you very much for your help.