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

geotargets: extending targets to work with geospatial data #675

Open
12 of 29 tasks
njtierney opened this issue Nov 29, 2024 · 22 comments
Open
12 of 29 tasks

geotargets: extending targets to work with geospatial data #675

njtierney opened this issue Nov 29, 2024 · 22 comments

Comments

@njtierney
Copy link
Contributor

njtierney commented Nov 29, 2024

Submitting Author Name: Nicholas Tierney
Submitting Author Github Handle: @njtierney
Other Package Authors Github handles: @Aariq @brownag
Repository: https://github.com/njtierney/geotargets
Version submitted: 0.2.0.9000
Submission type: Standard
Editor: @ldecicco-USGS
Reviewers: @amart90, @lidefi87

Due date for @amart90: 2025-01-09

Due date for @lidefi87: 2025-01-13
Archive: TBD
Version accepted: TBD
Language: en


  • Paste the full DESCRIPTION file inside a code block below:
Package: geotargets
Title: 'Targets' Extensions for Geospatial Formats
Version: 0.2.0.9000
Authors@R: c(
    person(
      given = "Nicholas", 
      family = "Tierney", 
      email = "[email protected]", 
      role = c("aut", "cre"),
      comment = c(ORCID = "0000-0003-1460-8722")
      ),
    person(
      given = "Eric",
      family = "Scott",
      role = c("aut"),
      comment = c(ORCID = "0000-0002-7430-7879")
    ),
    person(
      given = "Andrew",
      family = "Brown",
      role = c("aut"),
      comment = c(ORCID = "0000-0002-4565-533X")
    )
    )
Description: Provides extensions for various geospatial file formats, 
  such as shapefiles and rasters. Currently provides support for the 'terra'
  geospatial formats. See the vignettes for worked examples, demonstrations, 
  and explanations of how to use the various package extensions.
License: MIT + file LICENSE
Encoding: UTF-8
Language: en-GB
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.3.2
Imports: 
    targets (>= 1.8.0),
    rlang (>= 1.1.3),
    cli (>= 3.6.2),
    terra (>= 1.7.71),
    withr (>= 3.0.0),
    zip,
    lifecycle
Suggests: 
    crew (>= 0.9.2),
    knitr,
    ncmeta,
    rmarkdown,
    sf,
    stars,
    testthat (>= 3.0.0),
    fs
Config/testthat/edition: 3
URL: https://github.com/njtierney/geotargets, https://njtierney.github.io/geotargets/
BugReports: https://github.com/njtierney/geotargets/issues
VignetteBuilder: knitr

Scope

  • Please indicate which category or categories from our package fit policies this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.):

    • data retrieval
    • data extraction
    • data munging
    • data deposition
    • data validation and testing
    • workflow automation
    • version control
    • citation management and bibliometrics
    • scientific software wrappers
    • field and lab reproducibility tools
    • database software bindings
    • geospatial data
    • text analysis
  • Explain how and why the package falls under these categories (briefly, 1-2 sentences):

The geotargets package builds upon the targets package to facilitate geospatial computation, and skip up-to-date operations in geospatial pipelines. geotargets means the user doesn't need to write custom targets code to manage the special read/write/wrap/unwrap that is required when using packages like terra. It also facilitates dynamic branching, so a raster can be split into parts and then jobs farmed out in parallel and later combined.

  • Who is the target audience and what are scientific applications of this package?

The geotargets package is for people working with geospatial data with packages like terra. Workflows include analysis of sea ice changes over time (https://github.com/njtierney/icebreaker), through to creating statistical pipelines to estimate mosquito insecticide resistance (https://github.com/idem-lab/map-ir-pipeline).

The targets package does provide workflow automation, but in order for it to work with geospatial data it requires custom code. The geotargets package helps users continue working with targets with minimal changes to workflow. Other packages like chopin provide ways to parallelise spatial operations, but this seems distinct from automation, although related.

N/A

  • If you made a pre-submission inquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted.

N/A

  • Explain reasons for any pkgcheck items which your package is unable to pass.
── geotargets 0.2.0.9000 ─────────────────────────────────────────────────

✔ Package name is available
✔ has a 'codemeta.json' file.
✔ has a 'contributing' file.
✔ uses 'roxygen2'.
✔ 'DESCRIPTION' has a URL field.
✔ 'DESCRIPTION' has a BugReports field.
✔ Package has at least one HTML vignette
✔ All functions have examples.
✔ Package has continuous integration checks.
✔ Package coverage is 97.7%.
✖ R CMD check found 1 error.
✔ R CMD check found no warnings.
ℹ Function names are duplicated in other packages

ℹ Current status:
✖ This package is not ready to be submitted.

I could not find the R CMD Check error, and all checks pass on GitHub Actions - https://github.com/njtierney/geotargets/actions/workflows/R-CMD-check.yaml

Technical checks

Confirm each of the following by checking the box.

This package:

Publication options

  • Do you intend for this package to go on CRAN?

  • Do you intend for this package to go on Bioconductor?

  • Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:

MEE Options
  • The package is novel and will be of interest to the broad readership of the journal.
  • The manuscript describing the package is no longer than 3000 words.
  • You intend to archive the code for the package in a long-term repository which meets the requirements of the journal (see MEE's Policy on Publishing Code)
  • (Scope: Do consider MEE's Aims and Scope for your manuscript. We make no guarantee that your manuscript will be within MEE scope.)
  • (Although not required, we strongly recommend having a full manuscript prepared when you submit here.)
  • (Please do not submit your package separately to Methods in Ecology and Evolution)

We would be interested in submitting this to JOSS, if possible.

Code of conduct

@ropensci-review-bot
Copy link
Collaborator

Thanks for submitting to rOpenSci, our editors and @ropensci-review-bot will reply soon. Type @ropensci-review-bot help for help.

@ropensci-review-bot
Copy link
Collaborator

🚀

Editor check started

👋

@njtierney njtierney changed the title geotargets: extending targets to produce geospatial pipelines at scale geotargets: extending targets to work with geospatial data Nov 29, 2024
@ropensci-review-bot
Copy link
Collaborator

Checks for geotargets (v0.2.0.9000)

git hash: 567a9935

  • ✔️ Package name is available
  • ✔️ has a 'codemeta.json' file.
  • ✔️ has a 'contributing' file.
  • ✔️ uses 'roxygen2'.
  • ✔️ 'DESCRIPTION' has a URL field.
  • ✔️ 'DESCRIPTION' has a BugReports field.
  • ✔️ Package has at least one HTML vignette
  • ✔️ All functions have examples.
  • ✔️ Package has continuous integration checks.
  • ✔️ Package coverage is 97.7%.
  • ✔️ R CMD check found no errors.
  • ✔️ R CMD check found no warnings.
  • 👀 Function names are duplicated in other packages

(Checks marked with 👀 may be optionally addressed.)

Package License: MIT + file LICENSE


1. Package Dependencies

Details of Package Dependency Usage (click to open)

The table below tallies all function calls to all packages ('ncalls'), both internal (r-base + recommended, along with the package itself), and external (imported and suggested packages). 'NA' values indicate packages to which no identified calls to R functions could be found. Note that these results are generated by an automated code-tagging system which may not be entirely accurate.

type package ncalls
internal base 99
internal geotargets 37
internal utils 2
internal stats 1
imports terra 20
imports targets 12
imports rlang 3
imports withr 2
imports zip 2
imports cli NA
imports lifecycle NA
suggests sf 2
suggests stars 1
suggests crew NA
suggests knitr NA
suggests ncmeta NA
suggests rmarkdown NA
suggests testthat NA
suggests fs NA
linking_to NA NA

Click below for tallies of functions used in each package. Locations of each call within this package may be generated locally by running 's <- pkgstats::pkgstats(<path/to/repo>)', and examining the 'external_calls' table.

base

substitute (21), list (15), library (10), as.expression (6), format (5), nrow (4), c (3), lapply (3), options (3), seq_len (3), dirname (2), drop (2), for (2), message (2), as.symbol (1), call (1), file.path (1), floor (1), getOption (1), gsub (1), ifelse (1), list.files (1), mode (1), ncol (1), numeric_version (1), paste0 (1), seq (1), sqrt (1), switch (1), Sys.getenv (1), toupper (1), warning (1)

geotargets

get_gdal_available_driver_list (5), read (5), marshal (4), geotargets_repair_option_name (2), tar_rast_read (2), tar_rast_write (2), check_gdal_shz (1), check_pkg_installed (1), gdal_version (1), geotargets_option_get (1), geotargets_option_set (1), release_bullets (1), semicolon_paste (1), semicolon_split (1), set_window (1), tar_stars (1), tar_stars_proxy (1), tar_stars_raw (1), tar_terra_collection_raw (1), tar_terra_rast (1), tar_terra_sds (1), tar_terra_sprc (1), write (1)

terra

rast (4), wrap (4), gdal (3), getTileExtents (3), crs (2), ext (2), sds (1), sprc (1)

targets

tar_option_get (6), tar_format (4), tar_target_raw (2)

rlang

call2 (1), caller_env (1), is_integerish (1)

sf

st_drivers (2)

utils

zip (2)

withr

local_tempdir (2)

zip

unzip (2)

stars

read_ncdf (1)

stats

window (1)

NOTE: Some imported packages appear to have no associated function calls; please ensure with author that these 'Imports' are listed appropriately.


2. Statistical Properties

This package features some noteworthy statistical properties which may need to be clarified by a handling editor prior to progressing.

Details of statistical properties (click to open)

The package has:

  • code in R (100% in 9 files) and
  • 3 authors
  • 2 vignettes
  • no internal data file
  • 7 imported packages
  • 13 exported functions (median 25 lines of code)
  • 54 non-exported functions in R (median 9 lines of code)

Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
The following terminology is used:

  • loc = "Lines of Code"
  • fn = "function"
  • exp/not_exp = exported / not exported

All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the checks_to_markdown() function

The final measure (fn_call_network_size) is the total number of calls between functions (in R), or more abstract relationships between code objects in other languages. Values are flagged as "noteworthy" when they lie in the upper or lower 5th percentile.

measure value percentile noteworthy
files_R 9 52.3
files_vignettes 2 81.7
files_tests 9 85.9
loc_R 898 62.5
loc_vignettes 605 80.8
loc_tests 539 72.8
num_vignettes 2 85.5
n_fns_r 67 64.0
n_fns_r_exported 13 53.7
n_fns_r_not_exported 54 67.9
n_fns_per_file_r 4 59.7
num_params_per_fn 19 99.0 TRUE
loc_per_fn_r 12 36.7
loc_per_fn_r_exp 25 55.9
loc_per_fn_r_not_exp 10 31.5
rel_whitespace_R 10 46.5
rel_whitespace_vignettes 18 63.3
rel_whitespace_tests 7 47.7
doclines_per_fn_exp 206 98.7 TRUE
doclines_per_fn_not_exp 0 0.0 TRUE
fn_call_network_size 45 63.0

2a. Network visualisation

Click to see the interactive network visualisation of calls between objects in package


3. goodpractice and other checks

Details of goodpractice checks (click to open)

3a. Continuous Integration Badges

R-CMD-check.yaml
pkgcheck

GitHub Workflow Results

id name conclusion sha run_number date
12077595673 pages build and deployment success eeaa5f 47 2024-11-29
12077569956 pkgcheck success 1b7be7 5 2024-11-29
12077569953 pkgdown success 1b7be7 175 2024-11-29
12077569960 R-CMD-check success 1b7be7 250 2024-11-29
12077569950 test-coverage success 1b7be7 254 2024-11-29

3b. goodpractice results

R CMD check with rcmdcheck

rcmdcheck found no errors, warnings, or notes

Test coverage with covr

Package coverage: 97.7

Cyclocomplexity with cyclocomp

No functions have cyclocomplexity >= 15

Static code analyses with lintr

lintr found no issues with this package!


4. Other Checks

Details of other checks (click to open)

✖️ The following function name is duplicated in other packages:

    • set_window from JOPS


Package Versions

package version
pkgstats 0.2.0.48
pkgcheck 0.1.2.77


Editor-in-Chief Instructions:

This package is in top shape and may be passed on to a handling editor

@emilyriederer
Copy link

Thanks for the submission @njtierney ! We're excited to move forward on this review.

I'm happy to introduce @ldecicco-USGS to take it from here as editor.

@emilyriederer
Copy link

@ropensci-review-bot assign @ldecicco-USGS as editor

@ropensci-review-bot
Copy link
Collaborator

Assigned! @ldecicco-USGS is now the editor

@ldecicco-USGS
Copy link

@ropensci-review-bot check package

@ropensci-review-bot
Copy link
Collaborator

Thanks, about to send the query.

@ropensci-review-bot
Copy link
Collaborator

🚀

Editor check started

👋

@ldecicco-USGS
Copy link

Editor checks:

  • Documentation: The package has sufficient documentation available online (README, pkgdown docs) to allow for an assessment of functionality and scope without installing the package. In particular,
    • Is the case for the package well made?
    • Is the reference index page clear (grouped by topic if necessary)?
    • Are vignettes readable, sufficiently detailed and not just perfunctory?
  • Fit: The package meets criteria for fit and overlap.
  • Installation instructions: Are installation instructions clear enough for human users?
  • Tests: If the package has some interactivity / HTTP / plot production etc. are the tests using state-of-the-art tooling?
  • Contributing information: Is the documentation for contribution clear enough e.g. tokens for tests, playgrounds?
  • License: The package has a CRAN or OSI accepted license.
  • Project management: Are the issue and PR trackers in a good shape, e.g. are there outstanding bugs, is it clear when feature requests are meant to be tackled?

Editor comments

Package looks good! I'm looking for some reviewers who ideally already have this use case.


@ropensci-review-bot
Copy link
Collaborator

Checks for geotargets (v0.2.0.9000)

git hash: 3f8f1daa

  • ✔️ Package name is available
  • ✔️ has a 'codemeta.json' file.
  • ✔️ has a 'contributing' file.
  • ✔️ uses 'roxygen2'.
  • ✔️ 'DESCRIPTION' has a URL field.
  • ✔️ 'DESCRIPTION' has a BugReports field.
  • ✔️ Package has at least one HTML vignette
  • ✔️ All functions have examples.
  • ✔️ Package has continuous integration checks.
  • ✖️ Package coverage failed
  • ✖️ R CMD check found 1 error.
  • ✔️ R CMD check found no warnings.
  • 👀 Function names are duplicated in other packages

Important: All failing checks above must be addressed prior to proceeding

(Checks marked with 👀 may be optionally addressed.)

Package License: MIT + file LICENSE


1. Package Dependencies

Details of Package Dependency Usage (click to open)

The table below tallies all function calls to all packages ('ncalls'), both internal (r-base + recommended, along with the package itself), and external (imported and suggested packages). 'NA' values indicate packages to which no identified calls to R functions could be found. Note that these results are generated by an automated code-tagging system which may not be entirely accurate.

type package ncalls
internal base 101
internal geotargets 35
internal utils 2
internal stats 1
imports terra 20
imports targets 12
imports rlang 3
imports withr 2
imports zip 2
imports cli NA
imports lifecycle NA
suggests sf 2
suggests stars 1
suggests crew NA
suggests knitr NA
suggests ncmeta NA
suggests rmarkdown NA
suggests testthat NA
suggests fs NA
linking_to NA NA

Click below for tallies of functions used in each package. Locations of each call within this package may be generated locally by running 's <- pkgstats::pkgstats(<path/to/repo>)', and examining the 'external_calls' table.

base

substitute (21), list (15), library (10), as.expression (6), format (5), c (4), nrow (4), lapply (3), message (3), options (3), seq_len (3), dirname (2), drop (2), for (2), as.symbol (1), call (1), file.path (1), floor (1), getOption (1), gsub (1), ifelse (1), list.files (1), mode (1), ncol (1), numeric_version (1), paste0 (1), seq (1), sqrt (1), switch (1), Sys.getenv (1), toupper (1), warning (1)

geotargets

get_gdal_available_driver_list (5), read (5), marshal (4), geotargets_repair_option_name (2), tar_rast_read (2), tar_rast_write (2), check_gdal_shz (1), check_pkg_installed (1), check_user_resources (1), gdal_version (1), geotargets_option_get (1), geotargets_option_set (1), release_bullets (1), set_window (1), tar_stars (1), tar_stars_proxy (1), tar_stars_raw (1), tar_terra_collection_raw (1), tar_terra_rast (1), tar_terra_sds (1), write (1)

terra

rast (4), wrap (4), gdal (3), getTileExtents (3), crs (2), ext (2), sds (1), sprc (1)

targets

tar_option_get (6), tar_format (4), tar_target_raw (2)

rlang

call2 (1), caller_env (1), is_integerish (1)

sf

st_drivers (2)

utils

zip (2)

withr

local_tempdir (2)

zip

unzip (2)

stars

read_ncdf (1)

stats

window (1)

NOTE: Some imported packages appear to have no associated function calls; please ensure with author that these 'Imports' are listed appropriately.


2. Statistical Properties

This package features some noteworthy statistical properties which may need to be clarified by a handling editor prior to progressing.

Details of statistical properties (click to open)

The package has:

  • code in R (100% in 9 files) and
  • 3 authors
  • 2 vignettes
  • no internal data file
  • 7 imported packages
  • 13 exported functions (median 25 lines of code)
  • 52 non-exported functions in R (median 11 lines of code)

Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
The following terminology is used:

  • loc = "Lines of Code"
  • fn = "function"
  • exp/not_exp = exported / not exported

All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the checks_to_markdown() function

The final measure (fn_call_network_size) is the total number of calls between functions (in R), or more abstract relationships between code objects in other languages. Values are flagged as "noteworthy" when they lie in the upper or lower 5th percentile.

measure value percentile noteworthy
files_R 9 52.4
files_vignettes 2 81.7
files_tests 11 88.7
loc_R 897 62.5
loc_vignettes 605 80.8
loc_tests 577 74.0
num_vignettes 2 85.5
n_fns_r 65 63.3
n_fns_r_exported 13 53.8
n_fns_r_not_exported 52 66.9
n_fns_per_file_r 4 58.7
num_params_per_fn 19 99.1 TRUE
loc_per_fn_r 12 36.7
loc_per_fn_r_exp 25 55.9
loc_per_fn_r_not_exp 11 36.0
rel_whitespace_R 9 43.7
rel_whitespace_vignettes 17 62.0
rel_whitespace_tests 6 47.1
doclines_per_fn_exp 206 98.7 TRUE
doclines_per_fn_not_exp 0 0.0 TRUE
fn_call_network_size 49 64.6

2a. Network visualisation

Click to see the interactive network visualisation of calls between objects in package


3. goodpractice and other checks

Details of goodpractice checks (click to open)

3a. Continuous Integration Badges

R-CMD-check.yaml
pkgcheck

GitHub Workflow Results

id name conclusion sha run_number date
12078541372 pages build and deployment success eeaf5a 48 2024-11-29
12078514310 pkgcheck failure 3f8f1d 6 2024-11-29
12381252571 pkgdown success cf512c 184 2024-12-17
12381252575 R-CMD-check success cf512c 259 2024-12-17
12381252580 test-coverage success cf512c 263 2024-12-17

3b. goodpractice results

R CMD check with rcmdcheck

R CMD check generated the following error:

  1. checking tests ...
    Running ‘testthat.R’
    ERROR
    Running the tests in ‘tests/testthat.R’ failed.
    Last 13 lines of output:

old[17:21] vs new[17:21]

  [[4]]
       xmin      xmax      ymin      ymax 
  • 6.133333 6.533333 49.441667 49.816667
  • 6.141667 6.533333 49.441667 49.816667
  • Run testthat::snapshot_accept('tile-funs') to accept the change.
  • Run testthat::snapshot_review('tile-funs') to interactively review the change.

[ FAIL 3 | WARN 0 | SKIP 0 | PASS 49 ]
Error: Test failures
Execution halted

R CMD check generated the following test_fail:

  1. This file is part of the standard setup for testthat.

It is recommended that you do not modify it.

Where should you do additional test configuration?

Learn more about the roles of various files in:

* https://r-pkgs.org/testing-design.html#sec-tests-files-overview

* https://testthat.r-lib.org/articles/special-files.html

library(testthat)
library(geotargets)

test_check("geotargets")
▶ dispatched target opt
● completed target opt [0.004 seconds, 56 bytes]
▶ ended pipeline [0.101 seconds]
▶ dispatched target test_stars
● completed target test_stars [0.02 seconds, 49.9 kilobytes]
▶ ended pipeline [0.131 seconds]
▶ dispatched target test_stars_proxy
● completed target test_stars_proxy [0.02 seconds, 49.9 kilobytes]
▶ ended pipeline [0.135 seconds]
▶ dispatched target test_stars_mdim
● completed target test_stars_mdim [0.009 seconds, 8.276 kilobytes]
▶ ended pipeline [0.113 seconds]
Warning messages:
1: GDAL Error 6: SetIndexingVariable() not implemented
2: GDAL Error 6: SetIndexingVariable() not implemented
3: 1 targets produced warnings. Run targets::tar_meta(fields = warnings, complete_only = TRUE) for the messages.
▶ dispatched target test_stars_mdim_ncdf
● completed target test_stars_mdim_ncdf [0.007 seconds, 8.276 kilobytes]
▶ ended pipeline [0.123 seconds]
Warning messages:
1: GDAL Error 6: SetIndexingVariable() not implemented
2: GDAL Error 6: SetIndexingVariable() not implemented
3: 1 targets produced warnings. Run targets::tar_meta(fields = warnings, complete_only = TRUE) for the messages.
▶ dispatched target test_stars
● completed target test_stars [0.025 seconds, 49.9 kilobytes]
▶ dispatched target to_add
● completed target to_add [0.001 seconds, 55 bytes]
▶ dispatched branch test_stars_plus_e98a12d3e3374380
● completed branch test_stars_plus_e98a12d3e3374380 [0.002 seconds, 49.9 kilobytes]
▶ dispatched branch test_stars_plus_ba56ffbcd54ddba8
● completed branch test_stars_plus_ba56ffbcd54ddba8 [0.001 seconds, 49.9 kilobytes]
● completed pattern test_stars_plus
▶ ended pipeline [0.259 seconds]

Attaching package: 'targets'

The following object is masked from 'package:testthat':

matches

▶ dispatched target test_terra_rast
● completed target test_terra_rast [0.02 seconds, 7.992 kilobytes]
▶ ended pipeline [0.166 seconds]
▶ dispatched target rast1
▶ dispatched target rast2
● completed target rast1 [4.818 seconds, 9.92 kilobytes]
● completed target rast2 [5.228 seconds, 9.92 kilobytes]
▶ ended pipeline [8.18 seconds]
▶ dispatched target my_map
● completed target my_map [0.019 seconds, 7.992 kilobytes]
▶ dispatched target to_add
● completed target to_add [0 seconds, 55 bytes]
▶ dispatched branch my_map_plus_e98a12d3e3374380
● completed branch my_map_plus_e98a12d3e3374380 [0.006 seconds, 9.899 kilobytes]
▶ dispatched branch my_map_plus_ba56ffbcd54ddba8
● completed branch my_map_plus_ba56ffbcd54ddba8 [0.004 seconds, 9.902 kilobytes]
● completed pattern my_map_plus
▶ ended pipeline [0.255 seconds]
terra 1.8.5
▶ dispatched target r
● completed target r [0.026 seconds, 10.433 kilobytes]
▶ ended pipeline [0.217 seconds]
terra 1.8.5
terra 1.8.5
▶ dispatched target r
● completed target r [0.03 seconds, 30.385 kilobytes]
▶ ended pipeline [0.192 seconds]
▶ dispatched target raster_elevs
● completed target raster_elevs [0.148 seconds, 36.423 kilobytes]
▶ ended pipeline [0.296 seconds]
▶ dispatched target raster_elevs
● completed target raster_elevs [0.149 seconds, 19.597 kilobytes]
▶ ended pipeline [0.347 seconds]
terra 1.8.5
terra 1.8.5
▶ dispatched target my_file
● completed target my_file [0.002 seconds, 22.458 kilobytes]
▶ dispatched target my_map
● completed target my_map [0.008 seconds, 20.063 kilobytes]
▶ dispatched target rast_split_exts
● completed target rast_split_exts [0.011 seconds, 126 bytes]
▶ dispatched target rast_split_grid_exts
● completed target rast_split_grid_exts [0.01 seconds, 134 bytes]
▶ dispatched target rast_split_n_exts
creating 2 * 3 = 6 tile extents
● completed target rast_split_n_exts [0.077 seconds, 148 bytes]
▶ dispatched branch rast_split_c831794104ea3060
● completed branch rast_split_c831794104ea3060 [0.006 seconds, 7.034 kilobytes]
▶ dispatched branch rast_split_ae2c69362633e032
● completed branch rast_split_ae2c69362633e032 [0.074 seconds, 8.218 kilobytes]
▶ dispatched branch rast_split_2820724be1b2b6b4
● completed branch rast_split_2820724be1b2b6b4 [0.005 seconds, 5.509 kilobytes]
● completed pattern rast_split
▶ dispatched branch rast_split_grid_769150b2cab1932d
● completed branch rast_split_grid_769150b2cab1932d [0.005 seconds, 4.985 kilobytes]
▶ dispatched branch rast_split_grid_785ba1a1a2a0f45c
● completed branch rast_split_grid_785ba1a1a2a0f45c [0.005 seconds, 5.617 kilobytes]
▶ dispatched branch rast_split_grid_fad9f6a6bf9d122b
● completed branch rast_split_grid_fad9f6a6bf9d122b [0.006 seconds, 4.432 kilobytes]
▶ dispatched branch rast_split_grid_6c3d5fb6f33325c0
● completed branch rast_split_grid_6c3d5fb6f33325c0 [0.006 seconds, 6.076 kilobytes]
● completed pattern rast_split_grid
▶ dispatched branch rast_split_n_d8ba26f1a6cafd82
● completed branch rast_split_n_d8ba26f1a6cafd82 [0.006 seconds, 3.154 kilobytes]
▶ dispatched branch rast_split_n_e89d019cb4bd9bbf
● completed branch rast_split_n_e89d019cb4bd9bbf [0.006 seconds, 4.049 kilobytes]
▶ dispatched branch rast_split_n_ee7169a4778786f4
● completed branch rast_split_n_ee7169a4778786f4 [0.004 seconds, 3.761 kilobytes]
▶ dispatched branch rast_split_n_45f57cf91238850d
● completed branch rast_split_n_45f57cf91238850d [0.004 seconds, 2.608 kilobytes]
▶ dispatched branch rast_split_n_f18cead57eacacf2
● completed branch rast_split_n_f18cead57eacacf2 [0.003 seconds, 4.043 kilobytes]
▶ dispatched branch rast_split_n_b7d8a5ead900343f
● completed branch rast_split_n_b7d8a5ead900343f [0.003 seconds, 4.157 kilobytes]
● completed pattern rast_split_n
▶ ended pipeline [0.812 seconds]
terra 1.8.5
▶ dispatched target my_file
● completed target my_file [0.002 seconds, 7.994 kilobytes]
▶ dispatched target my_map
● completed target my_map [0.018 seconds, 12.346 kilobytes]
▶ dispatched target rast_split_exts
● completed target rast_split_exts [0.009 seconds, 144 bytes]
▶ dispatched branch rast_split_2c3887c5ffb0bb89
● completed branch rast_split_2c3887c5ffb0bb89 [0.007 seconds, 5.586 kilobytes]
▶ dispatched branch rast_split_48a36543737a276a
● completed branch rast_split_48a36543737a276a [0.077 seconds, 7.673 kilobytes]
▶ dispatched branch rast_split_ea79ef6e155b24a5
● completed branch rast_split_ea79ef6e155b24a5 [0.004 seconds, 1.255 kilobytes]
● completed pattern rast_split
▶ ended pipeline [0.366 seconds]
▶ dispatched target test_terra_vect
● completed target test_terra_vect [0.027 seconds, 117.643 kilobytes]
▶ dispatched target test_terra_vect_shz
● completed target test_terra_vect_shz [0.07 seconds, 33.695 kilobytes]
▶ ended pipeline [0.305 seconds]
▶ dispatched target my_vect
● completed target my_vect [0.013 seconds, 117.635 kilobytes]
▶ dispatched target to_sub
● completed target to_sub [0.001 seconds, 71 bytes]
▶ dispatched branch my_vect_subs_b54dbe7e6a683bc2
● completed branch my_vect_subs_b54dbe7e6a683bc2 [0.009 seconds, 9.89 kilobytes]
▶ dispatched branch my_vect_subs_a83a4aaf085c5a21
● completed branch my_vect_subs_a83a4aaf085c5a21 [0.005 seconds, 9.277 kilobytes]
● completed pattern my_vect_subs
▶ ended pipeline [0.25 seconds]
▶ dispatched target vect1
▶ dispatched target vect2
● completed target vect1 [4.067 seconds, 117.633 kilobytes]
● completed target vect2 [4.166 seconds, 117.633 kilobytes]
▶ ended pipeline [7.196 seconds]
[ FAIL 3 | WARN 0 | SKIP 0 | PASS 49 ]

══ Failed tests ════════════════════════════════════════════════════════════════
── Failure ('test-tar-terra-vect.R:29:3'): tar_terra_vect() works ──────────────
Snapshot of code has changed:
old[7:14] vs new[7:14]
extent : 5.74414, 6.528252, 49.44781, 50.18162 (xmin, xmax, ymin, ymax)
source : test_terra_vect
coord. ref. : lon/lat WGS 84 (EPSG:4326)

  • names : ID_1 NAME_1 ID_2 NAME_2 AREA POP
  • names : ID_1 NAME_1 ID_2 NAME_2 AREA POP
  • type :
  • type :
  • values : 1 Diekirch 1 Clervaux 312 18081
  • values : 1 Diekirch 1 Clervaux 312 1.808e+04
  •                  1 Diekirch     2 Diekirch   218 32543
    
  •                  1 Diekirch     2 Diekirch   218 3.254e+04
    
  •                  1 Diekirch     3  Redange   259 18664
    
  •                  1 Diekirch     3  Redange   259 1.866e+04
    
  • Run testthat::snapshot_accept('tar-terra-vect') to accept the change.
  • Run testthat::snapshot_review('tar-terra-vect') to interactively review the change.
    ── Failure ('test-tar-terra-vect.R:30:3'): tar_terra_vect() works ──────────────
    Snapshot of code has changed:
    old[7:14] vs new[7:14]
    extent : 5.74414, 6.528252, 49.44781, 50.18162 (xmin, xmax, ymin, ymax)
    source : test_terra_vect_shz} (test_terra_vect_shz)
    coord. ref. : lon/lat WGS 84 (EPSG:4326)
  • names : ID_1 NAME_1 ID_2 NAME_2 AREA POP
  • names : ID_1 NAME_1 ID_2 NAME_2 AREA POP
  • type :
  • type :
  • values : 1 Diekirch 1 Clervaux 312 18081
  • values : 1 Diekirch 1 Clervaux 312 1.808e+04
  •                  1 Diekirch     2 Diekirch   218 32543
    
  •                  1 Diekirch     2 Diekirch   218 3.254e+04
    
  •                  1 Diekirch     3  Redange   259 18664
    
  •                  1 Diekirch     3  Redange   259 1.866e+04
    
  • Run testthat::snapshot_accept('tar-terra-vect') to accept the change.

  • Run testthat::snapshot_review('tar-terra-vect') to interactively review the change.
    ── Failure ('test-tile-funs.R:9:5'): tile_n fails with non integer ─────────────
    Snapshot of code has changed:
    old[9:15] vs new[9:15]

    [[2]]
    xmin xmax ymin ymax

  • 6.133333 6.533333 49.816667 50.191667
  • 6.141667 6.533333 49.816667 50.191667
[[3]]
     xmin      xmax      ymin      ymax 

old[17:21] vs new[17:21]

[[4]]
     xmin      xmax      ymin      ymax 
  • 6.133333 6.533333 49.441667 49.816667
  • 6.141667 6.533333 49.441667 49.816667
  • Run testthat::snapshot_accept('tile-funs') to accept the change.
  • Run testthat::snapshot_review('tile-funs') to interactively review the change.

[ FAIL 3 | WARN 0 | SKIP 0 | PASS 49 ]
Error: Test failures
Execution halted

R CMD check generated the following check_fail:

  1. rcmdcheck_tests_pass

Test coverage with covr

ERROR: Test Coverage Failed

Cyclocomplexity with cyclocomp

No functions have cyclocomplexity >= 15

Static code analyses with lintr

lintr found no issues with this package!


4. Other Checks

Details of other checks (click to open)

✖️ The following function name is duplicated in other packages:

    • set_window from JOPS


Package Versions

package version
pkgstats 0.2.0.48
pkgcheck 0.1.2.77


Editor-in-Chief Instructions:

Processing may not proceed until the items marked with ✖️ have been resolved.

@Aariq
Copy link

Aariq commented Dec 18, 2024

The failing tests are due to updates to the data files included with terra. I'll update these snapshots.

@ldecicco-USGS
Copy link

@ropensci-review-bot seeking reviewers

@ropensci-review-bot
Copy link
Collaborator

Please add this badge to the README of your package repository:

[![Status at rOpenSci Software Peer Review](https://badges.ropensci.org/675_status.svg)](https://github.com/ropensci/software-review/issues/675)

Furthermore, if your package does not have a NEWS.md file yet, please create one to capture the changes made during the review process. See https://devguide.ropensci.org/releasing.html#news

@ldecicco-USGS
Copy link

@ropensci-review-bot assign @amart90 as reviewer

@ropensci-review-bot
Copy link
Collaborator

@amart90 added to the reviewers list. Review due date is 2025-01-09. Thanks @amart90 for accepting to review! Please refer to our reviewer guide.

rOpenSci’s community is our best asset. We aim for reviews to be open, non-adversarial, and focused on improving software quality. Be respectful and kind! See our reviewers guide and code of conduct for more.

@ropensci-review-bot
Copy link
Collaborator

@amart90: If you haven't done so, please fill this form for us to update our reviewers records.

@ldecicco-USGS
Copy link

@ropensci-review-bot assign @lidefi87 as reviewer

@ropensci-review-bot
Copy link
Collaborator

@lidefi87 added to the reviewers list. Review due date is 2025-01-13. Thanks @lidefi87 for accepting to review! Please refer to our reviewer guide.

rOpenSci’s community is our best asset. We aim for reviews to be open, non-adversarial, and focused on improving software quality. Be respectful and kind! See our reviewers guide and code of conduct for more.

@ropensci-review-bot
Copy link
Collaborator

@lidefi87: If you haven't done so, please fill this form for us to update our reviewers records.

@ldecicco-USGS
Copy link

I expect that with the holidays, we'll slide the due date out a week or so if needed.

@amart90
Copy link

amart90 commented Jan 3, 2025

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • Briefly describe any working relationship you have (had) with the package authors.
  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (if you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need: clearly stating problems the software is designed to solve and its target audience in README
  • Installation instructions: for the development version of package and any non-standard dependencies in README
  • Vignette(s): demonstrating major functionality that runs successfully locally
    • The code does not run without some modifications. See below for more details.
  • Function Documentation: for all exported functions
  • Examples: (that run successfully locally) for all exported functions
    • There are several examples that do not run. See below for more details.
  • Community guidelines: including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software have been confirmed.
  • Performance: Any performance claims of the software have been confirmed.
  • Automated tests: Unit tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.
    • Test coverage is good, but I think some more varied and robust could be implemented. More details below.
    • Two snapshot tests fail. See below for more details.
  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines.
    • Development status inconsistent.
    • Badges reading "failing" and "unknown"
    • Citation information not in README

Estimated hours spent reviewing: 10

  • Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer ("rev" role) in the package DESCRIPTION file.

Review Comments

Thank you for the invitation to review @ldecicco-USGS.

@njtierney, @Aariq, and @brownag: this is a great package and I have been excited for the solutions it brings to problems I've been working around for a couple of years. I learned a great deal in the process of reviewing your work and I am eager to further incorporate it into my own work. Overall, I found the package to work well, have great documentation and examples, and the code to be well written. I do have some notes for you to consider.

Vignettes

  • In the Raster metadata section of the Using terra with geotargets vignette, the return value for the line terra::units(tar_read(r)) of "" is different from what is returned when I run this locally, "m". This code should be reexamined and updated to ensure consistency.
  • Not all the code in the vignettes runs successfully. Specifically, code that the user should have populated in their _targets.R script files. Examples can be found in Using terra with geotargets and Example targets pipeline | Dynamic branching with raster tiles. These sections do not not produce a working targets pipeline, so later calls to tar_make(), tar_load(), etc. throw errors. You call attention that this code should be the contents of the _targets.R file as a comment at the top of the code chunks; however, I think that this is probably too subtle and might trip up even casual targets users. At the very least, I think this deserves a bit more instruction to the user at the first instance in each vignette about copying the code into a _targets.R script file, possibly giving them the suggestion to paste this into targets::tar_script(). Another possibility would be to wrap the code in this section in targets::tar_dir(targets::tar_script({ "code here" })) the same way it is coded in your examples. This is not the easiest for users to see examples, but it has the benefit of not altering the user's global state (i.e., possibly overwriting an existing _targets.R script file). Once that was addressed, the code in both vignettes ran as expected, with the exception of the case noted above.

Function documentation

  • In the documentation for tar_terra_tiles, the tar_terra_tiles call contains arguments that are inconsistent with the arguments available in this version of geotargets. That is, the example contains arguments for ncol and nrow but not tile_fun. This example should be updated.
  • As written, the example code in tar_terra_sds and tar_terra_sprc does not run successfully - it returns the error could not find function "tar_terra_sds/tar_terra_sprc". These examples should be rewritten. This could be solved by moving library(geotargets) within tar_script() or by adding the namespace to read geotargets::tar_terra_sds. The latter would be consistent with other example code that runs successfully, so that method is prefered.
  • In the example of the tile_grid documentation, there is a missing comma after the ) on the line following description = "Each tile is 2 blocks tall, 1 block wide" (line 299 of R/tar_terra_tiles.R). This results in an error.
  • The details section of the tar_terra_rast (and tar_terra_vect) documentation gives a nice brief explanation of the problem the function is solving and how it addresses it. Other tar_terra_* functions (such as tar_terra_sds) do not contain a similar explanation, and would benefit from having it, in accordance with the multiple points of enrty principle.

Interface

  • It might be helpful to enumerate possible options for the geotargets options that have limited possible values. For example, according to the documentation, terra_preserve_metadata can be either "drop" or "zip"; however, I am unconstrained to these options and I am able to set that option to any string. This might be the only option that this comment applies to, as the others are related to external GDAL options. This could handled within the tar_terra_*() functions themselves, but it would probably be better implemented in geotargets_option_set(). You could enumerate the options in the function parameters and use rlang::arg_match() or you could use some other internal logic within the function to ensure the value is not outside the set of possible choices.

Tests

  • I received a test failure for "tar_terra_vect() works" that "Snapshot of code has changed". The cause is that the POP column in the snapshot is class integer whereas the test code produces a SpatVector where the POP column is class numeric. This happened for both tar_terra_vect() snapshot tests (lines 29 and 30 of test-tar-terra-vect.R).
  • There could be additional testing (of course, there could always be more testing), particularly in the tile utility functions. Here are some examples I thought might be useful.
    • In test-tile-funs.R add tests for tile_blocksize and tile_grid. Additional tests could include that the expected number of tiles are being output and the sum of the number of cells is equal to the number of cells in the input raster.
    • In the test "recombined tiles are equal to original", it could be helpful to repeat the test for all 3 tile helper functions.
    • For the tests "tar_terra_sprc/sds() works", you have expectations for the class and snapshot. It might also be helpful to have tests to ensure some of the important metadata comes along, such as making sure the layer names, units, time, variables, and other attributes come along when expected. The snapshots for these tests do not include those metadata, so this may be as easy as assigning some of these attributes in the test SPRC/SDS and adding those fields in the snapshot.

Functionality

  • There is still a problem with some of the SpatRaster attributes not being preserved in in SpatRaster targets using tar_terra_rast() with geotargets::geotargets_option_set(terra_preserve_metadata = "zip"): namely, the varnames and longnames attributes.
        targets::tar_dir({
    targets::tar_script({
        geotargets::geotargets_option_set(terra_preserve_metadata = "zip")
        modify_rast <- function(r) {
            r <- c(r, r + 2, r * 2)
            names(r) <- c("elev", "elev_p_2", "elev_t_2")
            terra::units(r) <- "m"
            terra::time(r) <- as.Date(c("2020-01-01", "2020-01-02", "2020-01-03"))
            terra::varnames(r) <- "elev"
            terra::longnames(r) <- "elevation above sea level in meters"
            r
        }
        
        list(
            geotargets::tar_terra_rast(
                elev_rast,
                terra::rast(system.file("ex/elev.tif", package="terra"))
            ),
            geotargets::tar_terra_rast(
                modified_rast,
                modify_rast(elev_rast)
            )
        )
    })
    
    targets::tar_make()
    modified_rast <- targets::tar_read(modified_rast)

})

I tried using different gdal raster drivers and none of them preserved the varnames or longnames attributes. To me this is not a reason to stall the review, but I do think that a this should be logged as an issue to eventually resolve and there should be a note in the documentation explaining to that these attributes are not preserved.

Other comments

  • Consider adding a year to the CITATION.cff file. Currently, citation("geotargets") returns a warning that it "could not determine year for ‘geotargets’ from package DESCRIPTION file". It could also be useful to add a date (either dateModified or datePublished?) to codemeta.json.
  • Two of the badges are currently displaying unexpected/unhelpful information:
    • codecov: unknown. I am not familiar with how the CI for this works, but the link from the badge seems to show a 97.57% code coverage for the most recent commit (855a5e3), so I'm not sure why the badge shows "unknown".
      -pkgcheck: failing. I'm not very familiar with pkgcheck, but looking at the logs it seems like this is a problem with the CI and not a problem with the package contents.
  • Per 1.6 README | rOpenSci Packaging Guide that the users should be directed to citation information in the README. While GitHub provides a nice "Cite this repository" popup on the right-hand menu that is generated from the CITATION.cff file, there is nothing in the README.
  • The development status in codemeta.json ("https://www.repostatus.org/#wip") does not match the development status in the readme ("www.repostatus.org/#active"). I think this discrepancy is carried forward to Targetopia, which shows the status as "WIP".
  • There were about 70 instances of lines being greater than 80 characters in width. Most of these were cases where shortening the lines would have made the code more difficult to read (i.e., in code in the examples or in the default parameters in function definitions), but there were cases where the lines could be reduced in width. These cases were mostly just beyond 80 characters in function documentation or in-line comments. While not a requirement, it may be worth using lintr to locate these and shorten as many of these lines as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants