Add gsw #354
Add gsw #354
Conversation
|
Hi! Why removing the years parameters? It might also be a good idea to include it in the get function. Currently one would always download and calculate the complete time series? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #354 +/- ##
==========================================
+ Coverage 86.39% 86.76% +0.37%
==========================================
Files 64 66 +2
Lines 2756 2841 +85
==========================================
+ Hits 2381 2465 +84
- Misses 375 376 +1 ☔ View full report in Codecov by Sentry. |
|
@goergen95 I removed them only from the calc function. The user can still select which years to process while setting up the portfolio. |
|
Ah, yep! Sorry, did not catch that. 👍 |
|
Now that I looked into it, I noticed that we're not really consistent within the package with that logic, though. |
|
GFW are single layers with no temporal dimension. The temporal information is in the pixel's value, not the layers. However, I would like to see it consistent with other resources/indicators, aka in relation to humanfootprint. One could argue that one might be downloading all available layers in one sweep, but for certain analysis one might only calculate these indicators for a temporal subset. That would mean to expose the years argument in both functions. |
|
Ah, true. That makes sense then. |
data-raw/gsw_time_series.R
Outdated
| library(terra) | ||
|
|
||
| years <- 2000:2001 | ||
| outdir <- system.file("resources", package = "mapme.indicators") |
There was a problem hiding this comment.
This needs changing to reference mapme.biodiversity
data-raw/gsw_time_series.R
Outdated
| gsw_time_series <- prep_resources(x) | ||
| gsw_time_series <- gsw_time_series [[1]] | ||
|
|
||
| outdir <- "inst/resources/gsw_time_series" |
There was a problem hiding this comment.
This should reference 'res' instead of 'resources'
| @@ -0,0 +1,34 @@ | |||
| test_that("get_gsw_time_series works", { | |||
There was a problem hiding this comment.
Could this include a test to see that a footprints object is returned if successful?
|
@karpfen: the code in the example fails |
No description provided.