Fix live extraction on every dataset; release 0.1.0 - #13
Open
max578 wants to merge 37 commits into
Open
Conversation
added 13 commits
September 7, 2026 22:36
…tion The point route passed the point set as a GeoJSON object inside constantValue, which the Expression grammar reads as a Dictionary, so every live collect_gee_data() call failed with HTTP 400 and returned NA. Build the collection as Collection over Feature invocations with GeometryConstructors.Point geometries, the form the service accepts. Also: pass service and condition text to cli as data, never as a template (braces and backticks in the 400 message hid the cause); abort instead of aligning by position when a reply lacks point_id; abort on an empty project instead of forming projects// URLs; warn when gee_auth() falls back to the refused earthengine-legacy project.
…tests Record computeFeatures success, 401 and a masked-point reply from live Earth Engine calls (project redacted); the 403 and 429 cassettes are hand-authored from Google's documented error shapes and say so in their files. Add test-live.R, skipped unless GEEFETCH_LIVE=1 and GEEFETCH_PROJECT is set, covering SRTM and SMAP point extraction and one raster read. setup.R keeps those variables when live tests are requested.
…resampling warning .build_grid() warned that a region was resampled to fit the 2048x2048 pixel cap, but never said the dataset's native scale, the scale lost by resampling, or how to avoid it. The warning now states the native scale in metres, the effective scale after resampling, and suggests a smaller region. read_gee() gains a "Region size and resampling" section documenting this behaviour; max_dim itself stays internal, as no read_gee()/read_*() alias exposes it as an argument.
… through a valued raster Two testthat warnings, no failures: the SRTM dispatch test emitted the region-resampling warning unasserted, and the cache-routing test wrote a valueless SpatRaster template to .cache_set(), which cannot be written as a GeoTIFF and so warned on the disk-cache fallback path. The SRTM test now asserts the warning explicitly with expect_warning(). The cache-routing test now pre-populates the cache with a raster that carries real cell values, since .rest_compute_pixels() always returns one from real GEE response bytes; a template with no values never reaches .cache_set() outside a test fixture. The disk-cache-write fallback itself keeps independent coverage in a new test-cache.R case.
…alid HTML collect_gee_data()'s @returns \describe{} used a bare \item{<dataset columns>}{...}. Roxygen2's markdown parser read the angle brackets as an inline HTML tag and emitted \if{html}{\out{<dataset columns>}} verbatim, which R CMD check's HTML manual validation flagged: "<dataset> is not recognized" and "trimming empty <dt>", because a real HTML parser discards the unrecognised tag, leaving the \describe{} term empty. Wrapping the placeholder in backticks makes roxygen2 treat it as an inline code span, emitting \verb{<dataset columns>} instead, which Rd-to-HTML escapes as literal text in every output format. man/ collect_gee_data.Rd is hand-edited to this exact form rather than regenerated, to avoid the unrelated reformatting a full devtools::document() run under roxygen2 8.1.0 would apply to every Rd file and NAMESPACE; the change was verified against roxygen2 8.1.0's actual output on an isolated test package before being applied here.
…ng-warning uplift Adds a development-version section above 0.0.0.9001 describing, in user language, the five live-path fixes already committed in b494e03 (FeatureCollection point extraction, verbatim service error text, point_id-required alignment, empty-project abort, legacy-project warning) plus this session's actionable region-resampling warning.
…-2 and Landsat work live A live sweep of all 19 registered datasets found three defect classes that the mocked suite could not see: - The seven SLGA datasets pointed at CSIRO/SLGA, which is an ImageCollection; Image.load rejected it on both routes. Each dataset now names its attribute image (CSIRO/SLGA/CLY, ..., CSIRO/SLGA/pHc). - The Sentinel-2 cloud mask called Image.Or; the algorithm is Image.or. - No spatial filter preceded Collection.first, so scene-based collections (Sentinel-2, Landsat) returned an arbitrary scene, and the point route never applied QA masks, depth/stat, or computed NDVI. Add .ee_dataset_image(), used by read_gee() handlers, .rest_extract_*() and collect_gee_data(): load, date window, Filter.intersects on the request geometry (BBox for rasters, MultiPoint for points), mosaic for scene collections and first for global products, per-dataset QA mask, band resolution (SLGA depth/stat, WorldClim variable), scale/offset, and NDVI with fill pixels masked and the band renamed. collect_gee_data() now forwards depth and stat and keys the cache on them. Algorithm names checked against the live algorithms listing; the masked-point cassette re-recorded for the new request body.
…sts for SLGA and Sentinel-2 httr2 1.3.0 no longer imports jsonlite, yet every resp_body_json() call needs it; April's audit had removed it as a dead dependency. R CMD check flagged the recorded cassette paths as over 100 bytes. The REST base URL is now read through .gee_rest_base() so the test setup can point replay at a short host; cassette directories and the fake project name are shortened to match. Live tests keep the real host. Ignore .DS_Store in the build. Regenerate man/ with roxygen2 8.1.0 (the multi-line importFrom layout is the only unrelated change). NEWS lists the dataset fixes.
…xtraction; wrap long cli lines
…der and point route
max578
force-pushed
the
fix/point-route-featurecollection
branch
from
September 7, 2026 13:06
4380c00 to
5176bb0
Compare
adamhsparks
requested changes
Sep 8, 2026
adamhsparks
left a comment
adamhsparks
left a comment
Member
There was a problem hiding this comment.
Using some automated checks locally these issues are flagged.
Using {checktor}
♥ R 4.6.1> [I] results <- checktor::checktor()
── Package Doctor - Diagnostic Report ───────────────────────────────────────────────────────────────────────────────────────────────────────────── v0.1.0 ──
Examining package at: .
── Code Health Check ──
✔ No `T`/`F` usage found
✔ No hardcoded seed setting found
✖ Potential unsuppressable `print()`/`cat()` usage
• collect_gee_data.R:475
Treatment: Use {.code message()} or {.code if(verbose)} conditions
✔ Option changes appear to be properly reset
✔ No obvious home directory writing detected
✖ Temp files without apparent cleanup
• setup.R:33
Treatment: Add cleanup (unlink, on.exit, withr::local_tempfile, ...)
✔ No `.GlobalEnv` modification detected
✔ No `installed.packages()` usage found
✔ No `options(warn = -1)` usage found
✔ No software installation in functions detected
✔ Core usage appears limited appropriately
✔ No `library()`/`require()` calls in package code
✔ `Sys.setenv()` calls appear to be reset
── DESCRIPTION File Health Check ──
! Potential software name formatting issues
• Description: Python should be in single quotes
! Potential unexplained acronyms: "GEE, REST, GRDC"
Treatment: Consider explaining these acronyms
✔ License formatting appears correct
✔ Title appears to be in Title Case
✔ Title length is within the 65-character guideline
✔ Title does not start with an article
✔ Title is free of redundant phrases
! Function names are single-quoted (reserve quotes for software names)
• Description: function name 'read_gee()' should not be quoted
• Description: function name 'collect_gee_data()' should not be quoted
Treatment: Drop the single quotes around function names like 'fn()'
✔ `Authors@R` field found
! `Authors@R` has no `[cph]` (copyright holder)
• Authors@R lacks any [cph] (copyright holder) role
Treatment: Add role 'cph' to a person, e.g. role = c('aut','cre','cph')
ℹ No references found in Description
✔ Description length appears adequate
✔ Description does not start with a forbidden phrase
✔ Description quotes 'R' properly
✔ LICENSE year looks current
── Documentation Health Check ──
✔ All function documentation has `\value` tags
✔ Exported functions include `\examples`
ℹ Roxygen2 usage detected - ensure to run `roxygenize()` before submission
✔ Example structure appears appropriate
! Commented-out code in `\examples`
• gee_auth.Rd: commented-out call in \examples
• read_gee.Rd: commented-out call in \examples
Treatment: Remove the comment or make it a real example
✔ Unexported examples use `:::` where needed
✔ `\dontrun` use is appropriate
✔ Examples guard Suggested-package usage
── General Health Check ──
✔ Package size: "0.32 MB" (under 5 MB limit)
✔ No obvious URL issues found
✔ NEWS file found
✔ README relative links resolve to shipped files
── CRAN Policy Violations Check ──
✔ No `browser()` calls found
✔ No dangerous system calls found
! Potential file operations without temp directory
• cache.R:195 (saveRDS())
Treatment: Ensure file operations use temporary directories
✔ Network operations appear properly wrapped
── Diagnosis Summary ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
✖ Found 12 issues across 8 failed checks
Review the detailed diagnosis above for specific remedies.
Use `prescribe()` to get treatment recommendations.
── Recommended Next Steps
1. Apply the treatments suggested above
2. Run `devtools::check()` for standard R CMD check
3. Re-run `checktor()` to verify treatments
4. Submit to CRAN when diagnosis is clean
✗ R 4.6.1> [I] checktor::prescribe(results)
── Treatment Recommendations ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
── Unsuppressable Output Issues
Treatment: Use {.code message()} or gate output on a verbose parameter
# Before
print('Processing...')
# After - option 1
message('Processing...')
# After - option 2
my_function <- function(data, verbose = TRUE) {
if (verbose) cli::cli_inform('Processing...')
}Using {goodpractice}.
♥ R 4.6.1> [I] goodpractice::gp()
── Preparing goodpractice for geefetch ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
✔ Preparing: description [8ms]
✔ Preparing: code_structure [4ms]
✔ Preparing: namespace [5ms]
Registered S3 method overwritten by 'data.table':
method from
print.data.table colorDF
✔ Preparing: covr [16.6s]
✔ Preparing: cyclocomp [24.8s]
✔ Preparing: package_structure [5ms]
✔ Preparing: lintr [4.8s]
✔ Preparing: rcmdcheck [1m 7.4s]
✔ Preparing: rd [55ms]
✔ Preparing: revdep [262ms]
✔ Preparing: roxygen2 [288ms]
✔ Preparing: spelling [475ms]
ℹ Package geefetch
ℹ Checking that VignetteBuilder package knitr is installed.
✔ VignetteBuilder package knitr is installed.
✔ Preparing: urlchecker [6.4s]
Chunk start/end indices failed sanity checks in vignette.
Chunk start/end indices failed sanity checks in vignette.
Chunk start/end indices failed sanity checks in vignette.
✔ Preparing: vignette [20ms]
── It is good practice to ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
✖ keep functions short and focused. Long functions are harder to understand, test, and maintain. Consider splitting into smaller helpers. (Default limit of
50 set with option 'goodpractice.function_length_limit').
R/auth.R:110
R/auth.R:249
R/auth.R:325
R/backend_rest.R:309
R/backend_rest.R:385
... and 5 more lines
✖ remove or use internal functions that are defined but never called. Dead code increases maintenance burden.
R/backend_rest.R:285
R/backend_rest.R:637
R/backend_rest.R:686
R/collect_gee_data.R:385
R/qa_masking.R:138
... and 1 more line
✖ write unit tests for all functions, and all package code in general. 88% of code lines are covered by test cases.
R/auth.R:125
R/auth.R:126
R/auth.R:127
R/auth.R:128
R/auth.R:147
... and 183 more lines
✖ write short and simple functions. These functions have high cyclomatic complexity (>15): collect_gee_data (31). You can make them easier to reason about
by encapsulating distinct steps of your function into subfunctions.
✖ avoid long code lines, it is bad for readability. Also, many people prefer editor windows that are about 80 characters wide. Try making your lines shorter
than 80 characters
R/auth.R:391:81
R/auth.R:394:81
R/backend_rest.R:9:81
R/collect_gee_data.R:161:81
R/collect_gee_data.R:456:81
... and 57 more lines
✖ use `anyNA()` instead of `any(is.na(x))`. It is more efficient because it stops at the first `NA`.
R/utils.R:89:25
✖ avoid passing the same argument twice in a function call. Duplicate arguments are silently ignored, which is likely a bug.
R/auth.R:142:7
R/auth.R:158:7
R/auth.R:208:9
R/auth.R:216:9
R/backend_rest.R:330:7
... and 24 more lines
✖ use `fixed = TRUE` in `grep()`/`grepl()`/ `sub()`/`gsub()` when the pattern is a plain string, not a regular expression. It is faster and clearer.
R/backend_rest.R:468:33
R/external_facts.R:39:20
tests/testthat/test-coverage_boost2.R:8:21
tests/testthat/test-coverage_boost2.R:9:21
tests/testthat/test-coverage_boost2.R:10:21
... and 2 more lines
✖ use `paste0()` instead of `paste(..., sep = '')` and `file.path()` instead of `paste(..., sep = '/')`.
R/backend_rest.R:415:10
R/external_facts.R:149:20
✖ avoid operators like `<<-` (global assignment) and `:::` (accessing internal package functions). These make code harder to understand and maintain.
tests/testthat/test-backend_rest-http.R:41:21
tests/testthat/test-backend_rest-http.R:44:30
tests/testthat/test-backend_rest-http.R:45:19
tests/testthat/test-backend_rest-http.R:66:15
tests/testthat/test-backend_rest-http.R:86:15
... and 3 more lines
✖ avoid `c()` with a single argument or no arguments. `c('x')` is just `'x'`, and `c()` is better written as `NULL`.
R/external_facts.R:88:53
tests/testthat/test-validation.R:89:26
tests/testthat/test-validation.R:96:24
tests/testthat/test-validation.R:103:32
tests/testthat/test-validation.R:122:45
✖ split `expect_true(a && b)` into separate `expect_true(a)` and `expect_true(b)` calls. Separate assertions give clearer failure messages.
tests/testthat/test-coverage_boost.R:86:3
✖ use `expect_gt(x, y)` instead of `expect_true(x > y)`. Dedicated comparison expectations give better failure messages.
tests/testthat/test-cache.R:10:3
tests/testthat/test-cache.R:71:3
tests/testthat/test-cache.R:93:3
tests/testthat/test-coverage_boost.R:41:3
tests/testthat/test-coverage_boost.R:87:3
... and 12 more lines
✖ prefer `expect_identical()` over `expect_equal()` when the comparison should be exact. `expect_equal()` uses `all.equal()` with tolerance, which can mask
subtle type or attribute differences.
tests/testthat/test-auth.R:50:3
tests/testthat/test-cache.R:8:3
tests/testthat/test-cache.R:16:3
tests/testthat/test-cache.R:37:3
tests/testthat/test-cache.R:42:3
... and 121 more lines
✖ use `expect_length(x, n)` instead of `expect_equal(length(x), n)`. The dedicated function gives clearer failure messages.
tests/testthat/test-cache.R:77:3
tests/testthat/test-collect_gee_data.R:146:3
tests/testthat/test-coverage_boost2.R:148:3
tests/testthat/test-handler_registry.R:158:3
tests/testthat/test-handler_registry.R:159:3
... and 1 more line
✖ use `expect_named(x, expected)` instead of `expect_equal(names(x), expected)`. The dedicated function gives clearer failure messages.
tests/testthat/test-handler_registry.R:160:3
✖ use `expect_null()` instead of `expect_equal(x, NULL)` or `expect_identical(x, NULL)`. The dedicated function gives clearer failure messages.
tests/testthat/test-expression_builder.R:11:3
✖ use `expect_s3_class(x, 'class')` instead of `expect_equal(class(x), 'class')`. The dedicated function gives clearer failure messages and handles multiple
classes correctly.
tests/testthat/test-coverage_boost2.R:159:3
tests/testthat/test-validation.R:157:3
✖ use `expect_true()` instead of `expect_equal(x, TRUE)` and `expect_false()` instead of `expect_equal(x, FALSE)`. The dedicated functions give clearer
failure messages.
tests/testthat/test-expression_builder.R:10:3
✖ use `expect_type(x, 'type')` instead of `expect_equal(typeof(x), 'type')`. The dedicated function gives clearer failure messages.
tests/testthat/test-auth.R:23:3
tests/testthat/test-auth.R:24:3
tests/testthat/test-auth.R:25:3
tests/testthat/test-dispatcher.R:56:3
✖ use `@inheritParams` to avoid duplicating parameter documentation. Identical `@param` descriptions across files suggest shared docs should be inherited
from a single source.
R/backend_rest.R:514
R/backend_rest.R:561
R/backend_rest.R:637
R/backend_rest.R:686
R/collect_gee_data.R:260
... and 60 more lines
✖ fix spelling errors in package documentation. Use `spelling::spell_check_package()` to review misspelled words and add false positives to inst/WORDLIST.
read_era5.Rd:43
NEWS.md:105
NEWS.md:155
read_worldclim.Rd:53
read_worldclim.Rd:54
... and 136 more linesFrom these checks, I'd certainly remove dead code and lint the tests.
Using @inheritParams is also wise.
I note that this suggests using message() over print() and while I agree, I don't understand why we're not using {cli} here as it's an Import.
In cache.R, line 64 uses rlang::is_installed(), which is already imported, it does not need to be namespaced. I didn't check but there are likely to be other instances like this that should not be namespaced if the function is already imported.
For the cached files, also check and benchmark the use of {qs2}, I'm using it in the PRIM tool for fast read access of spatial data files in the Shiny web app.
Also with the cached files, be careful writing to the user space. I gave up on this for the {getCRUCLdata} because of issues with CRAN and the directory that I used. I only cache in-session using {httr2}.
added 14 commits
September 8, 2026 12:52
…lled .ee_reduce_regions, .rest_extract_raster, .rest_extract_points (backend_rest.R), .rest_extract_single_point (collect_gee_data.R), and .ee_normalized_difference (qa_masking.R) were never called from any live code path; the actual extraction routes go through .rest_extract_raster_expr (handlers.R) and .rest_extract_batch_points / .safe_extract_points_batch (collect_gee_data.R), and the S2/Landsat NDVI computation is inline in expression_dataset.R. Drop the dead functions, their now-orphaned tests, and correct a code comment in external_facts.R that named the deleted qa_masking.R function.
.print_collection_info() called print(info_dt) directly, an unsuppressable print() outside any if()/verbose guard checktor flags. Format the dataset table as plain text with a new .format_dataset_table() helper and emit it via cli::cli_verbatim() instead.
Disk caching under tools::R_user_dir() previously ran unconditionally whenever cache = TRUE (the default), writing to the real user cache directory in normal use and, because tests/testthat/setup.R redirected a cache-dir option under the wrong (dotted) name, in most test runs too. .cache_get()/.cache_set() now persist to disk only when options(geefetch.cache.disk = TRUE) is set, via a new gee_cache_disk() helper; the in-session memory cache is unchanged and remains the default. gee_clear_cache() clears the memory store unconditionally and the disk store when present. setup.R's redirect now uses the option names the code actually reads, and its temp cache directory is removed at teardown. Also: single-quote 'Python', drop quotes around read_gee()/ collect_gee_data() in DESCRIPTION, expand GEE/REST/GRDC on first use, add the "cph" role (DESCRIPTION already names "geefetch authors" as copyright holder in LICENSE); reword two roxygen example comments whose parenthetical asides tripped checktor's commented-code heuristic (no code was actually commented out).
utils.R: any(is.na(dates)) -> anyNA(dates).
backend_rest.R: fixed = TRUE for the two plain-string grepl()/gsub()
patterns; the REST request URL builder now uses file.path() instead
of paste0() with literal slashes.
external_facts.R: toString() replaces paste(..., collapse = ", ")
for the bands fact value; a single-element c("B") becomes "B".
Tests: fixed = TRUE on grepl() calls matching a plain string; drop
c() around single numeric literals in test-validation.R.
Converts expect_equal()/expect_true() calls goodpractice flagged to the dedicated forms: expect_identical() for exact comparisons (~120 sites), expect_gt/gte/lt/lte() for comparisons, expect_length(), expect_named(), expect_null(), expect_s3_class(), expect_true()/expect_false(), and expect_type(). Splits one expect_true(a && b) into separate assertions. Two expect_equal() calls in test-validation.R are deliberately left as expect_equal(): seq.Date() returns an integer-backed Date while as.Date() returns a double-backed one, so expect_identical() fails on storage mode even though the dates are equal; a comment records why. Drops geefetch::: qualification in test-backend_rest-http.R and test-catalogue-diff.R -- internal functions are already visible to testthat via devtools::load_all().
Adds an internal .gee_shared_params() (R/geefetch-package.R, @Keywords internal so @inheritParams can resolve it) documenting the parameters goodpractice found duplicated verbatim across files: did, meta, date, coords, region, bands, backend, cache, verbose, max_tries, initial_delay. All nine read_*() convenience wrappers, read_gee(), collect_gee_data(), and the internal request/cache helpers now `@inheritParams .gee_shared_params` instead of repeating the text; each function keeps only the @PARAM entries unique to it (variable, collection, depth, stat, dataset_id, ...). The cache @PARAM wording is updated in the one shared place to describe the memory-first/opt-in-disk model from the cache safety change. read_gee()'s backend explanation (no-Python REST vs server-side rgee) moves from its @PARAM into the function description, since it no longer has its own @PARAM block to carry it.
Functions already imported via importFrom (data.table, rbindlist, setcolorder; httr2's request/req_headers/req_body_json/req_retry/ req_perform/resp_status/resp_body_json/resp_body_raw; rlang's is_installed/arg_match) are now called without the pkg:: prefix, per the reviewer's note that a namespaced call to an already-imported function is redundant. Functions not imported (data.table::set, data.table::fifelse, httr2::req_error) are left namespaced. No importFrom lines added; NAMESPACE is unchanged. Three tests mocked is_installed() via local_mocked_bindings(..., .package = "rlang"), which only intercepts an explicit rlang::is_installed() call; now that the production code calls it bare (import-style), the mock must land in geefetch's own namespace instead, so .package = "rlang" is dropped from those calls per testthat's documented pattern for imported functions.
…lexity goodpractice flagged collect_gee_data() at cyclomatic complexity 31 (limit 15). Splits it into .cgd_resolve_datasets() (validate + resolve aliases), .cgd_classify_datasets() (time-series vs static), .cgd_build_scaffold() (the location x date data.table), .cgd_extract_timeseries()/.cgd_extract_static() (the two API-call loops, still mutating dt by reference via data.table::set()), and .cgd_finish() (column order + na.rm). collect_gee_data() itself drops to complexity 5; the extraction helpers, the most complex of the rest, are 7 and 5. Public signature and behaviour are unchanged; also drops a redundant pts_for_date <- coords copy in the time-series loop (coords is never mutated per-date, so the copy was always identical to coords).
Wraps every line goodpractice's lintr check flagged (>80 chars) across
R/ and tests/ (auto-generated httptest2 cassette fixtures under
tests/testthat/cf-*/ are left untouched -- they are recorded HTTP
response dumps, not hand-authored source). Long string literals are
split across paste0() calls or reformatted as multi-line calls; no
string content changes. Two test_that() titles that would otherwise
still run long move to paste0() rather than a raw embedded newline in
the title string. Two repeated helper-recapture.R comment lines and a
repeated "HTTP cassette missing" skip message (5 sites in
test-backend_rest-http.R) are reflowed; local_fake_auth() there also
gets a geefetch_env local variable instead of repeating
asNamespace("geefetch")$.geefetch_env three times.
spelling::spell_check_package() flagged 123 words not previously in the wordlist: dataset/product identifiers (MOD13A2, LC09, SRTMGL1, bio01-bio19, ...), acronyms (STAC, EOSDIS, GCP, MSI, DAAC, ...), author surnames from citations (Didan, Farr, Sabater, Rossel, Viscarra, Searle, Hijmans, Parra, Hulley, Nygard), numeric/unit tokens (1km, 30m, 2048x2048, 50N, ...), git/DOI hash fragments, and standard technical terms the dictionary lacks (backtick, codebase, dropdown, dispatchable, mosaicked, toolchain, defensibility, reflectances, Isothermality). None are misspellings; all are added to the wordlist rather than reworded. spelling::spell_check_package() now reports zero errors.
Adds a User-visible changes section to the 0.1.0 heading for the two behavioural changes from this review pass: disk caching is now opt-in via gee_cache_disk() / options(geefetch.cache.disk = TRUE), and the verbose progress table goes through cli instead of print().
…_shared_params .validate_dataset() (utils.R) was called only from its own two tests, never from any live code path -- the sixth uncalled internal function goodpractice's dead-code check reported in the original review (the other five were fixed in 1f1cba7); this pass's re-run of goodpractice surfaced it explicitly. Removed it and its now-orphaned tests. Adds @Rdname gee_shared_params to the shared-params dictionary function so goodpractice's "documented function needs @export/@noRd/ @Rdname" check passes; the generated Rd file renames from dot-gee_shared_params.Rd to gee_shared_params.Rd (the function's own \name{} is unchanged). goodpractice's dead-code check still flags .gee_shared_params itself as uncalled -- expected, since its only "caller" is roxygen2's @inheritParams resolution at document() time, not any runtime code path; noted as a residual in the review report.
…the remaining indentation and quote lints
…cument the shared-parameter topic's value; reword DESCRIPTION acronyms
added 5 commits
September 8, 2026 20:37
qs2::qd_save()/qd_read() write about 35x faster and read about 7x faster than gzip RDS on a 687k-row table, at a comparable size on disk, and beat fst on both speed and size. Rasters stay GeoTIFF via terra::writeRaster(), unchanged. Objects qdata can hold (vectors, lists, data.frames, matrices) use .qdata via qd_save()/qd_read(); anything else falls back to the general-purpose .qs2 via qs_save()/qs_read(). A shallow copy strips a data.table's .internal.selfref pointer before qd_save() so qdata's per-session externalptr warning never fires; setDT() restores it on read. The cache key now digests a .CACHE_FORMAT stamp alongside the dataset ID and parameters, so a format change (like this one) shifts every key and old on-disk files are simply missed rather than misread. gee_clear_cache() still sweeps old .fst/.rds files alongside the new extensions. Thread count for qs2 I/O is computed once per session: respects options(geefetch.threads) if set, otherwise detectCores() - 1 floored at 1, capped at 2 under _R_CHECK_LIMIT_CORES_ for CRAN checks. fst moves out of Suggests (no longer used); qs2 and parallel join Imports.
Add round-trip tests for a data.table with Date, character, integer and double columns, and for a numeric vector, asserting identical() survives the qdata write/read cycle including the .internal.selfref strip-and-restore. Add a test that an old-format (pre-format-stamp) .rds file at the path its key would have produced is missed, not read, once the current .cache_hash() includes .CACHE_FORMAT. Add a test that gee_clear_cache() still removes leftover .rds/.fst files alongside current-format ones. Rework the file-extension patterns in the existing gee_clear_cache() tests and the corrupt-file test for the new .qdata/.qs2 extensions; drop the now-meaningless "cache uses RDS when fst not available" test (fst is no longer part of the cache at all).
… of two, no core detection
… against The seven SLGA datasets sample attribute images inside CSIRO/SLGA, but the public catalogue publishes one record for the collection and none for the images, so the live catalogue check failed for all seven. Each entry now carries stac_id = "CSIRO/SLGA"; the check compares bands against that record and confirms the sampled image belongs to it, and gee_external_facts() cites it as the source. The sampled asset is unchanged.
Collaborator
Author
|
Thanks for the review and repair directions. Everything is applied on the branch except
Two items left as they are:
checktor now reports one item, a heuristic: it flags REST and GRDC as The repository flattening (#3) and precompiled vignettes (#1) will each come |
added 4 commits
September 9, 2026 11:56
Rename the check_types column of gee_external_facts() (was oracle_types) and the environment variable that enables the live catalogue checks (GEEFETCH_CATALOGUE_CHECK, was GEEFETCH_ORACLE); reword the comments that referred to internal review vocabulary.
The fallback message interpolated `{.GEE_DEFAULT_PROJECT}` directly. cli
3.4.0 and later read a brace expression starting with a dot as a style
name, not a value, so building the warning threw "Invalid cli literal"
and the call ended in an error instead of a warning. A first-time user
who runs gee_auth() before saving a project number hit that error.
Wrap the object in parentheses, the form already used for the project
line two calls below, and cover the path with a test that fails against
the old string.
…s it Splitting the extraction loop into .cgd_extract_timeseries() and .cgd_extract_static() moved the cli_progress_update() calls out of the frame that created the bar. cli resolves a bar by the environment that created it, so every update aborted with "Cannot find current progress bar", and collect_gee_data() failed for any run of more than one API call with its default verbose = TRUE. Keep the id returned by cli_progress_bar(), pass it to the helpers, and update and close by id. Every existing batch test passed verbose = FALSE, so add one that shows the bar.
computePixels returns a GeoTIFF with no band descriptions, so terra named the layer after the temporary file it was written to. That name reached the user in print() output and became the column name in terra::extract() results.
Collaborator
Author
|
I've pushed the PR, here is the description (three commits):
Each fix has a test that fails against the old code. R CMD check --as-cran is 0 errors, 0 warnings, 0 notes, and CI is green on the new head. |
collect_gee_data() counted one request per location, but the point route sends all the locations together in one request per dataset and date. For three locations over five dates with two datasets it announced 30 calls and made 10, and the progress bar three lines below said so. The estimate now uses the same expression that drives the bar, and says that locations are batched. The test compares the announced number with the calls actually made.
Collaborator
Author
|
One more commit: collect_gee_data() estimated API calls as dates x locations x datasets. The point route sends all locations in one call per dataset and date, so for three locations over five dates with two datasets it announced 30 and made 10. The progress bar right below said 10/10. The estimate now uses the same expression as the bar and says the locations are batched. The test compares the announced number with the calls actually made. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
collect_gee_data()failed on every live call (HTTP 400: the point set wassent as a Dictionary, not a FeatureCollection) and two cli template leaks hid
the cause. Seven SLGA datasets, Sentinel-2 and Landsat also failed or
returned an arbitrary scene on both routes. All 19 registered datasets now
return data live on the raster and point routes.
R/expression_dataset.R) now servesread_gee()and
collect_gee_data(): date window, spatial filter on the requestgeometry, mosaic for scene collections, per-dataset QA mask, SLGA
depth/stat, scale/offset, NDVI with fill pixels masked.
tools::R_user_dir(); cached tables stored with qs2, rasters as GeoTIFF;format version in the cache key.
jsonliteback in Imports (httr2 >= 1.3.0 no longer importsit), portable test fixture paths, a clearer resampling warning, NEWS.
print()replaced with cli, DESCRIPTIONwording,
@inheritParams, dedicated test expectations, lines wrapped,collect_gee_data()split into helpers.Type of change
What was verified, and how
Every defect was reproduced against the live service before the change and
re-checked after it. The point route was cross-checked against the official
Python client (earthengine-api 1.7.18): SMAP
soil_moisture_amat 145E 30Son 2022-06-01 gives 0.1736955 from geefetch and 0.1736955 from Python.
GEEFETCH_LIVE=1 GEEFETCH_PROJECT=... GEEFETCH_EMAIL=...)R CMD check --as-cran geefetch_0.1.0.tar.gzpkgdown::build_site()Algorithm names used by the builder were checked against the live
algorithmslisting (985 entries):Image.or,Filter.intersects,GeometryConstructors.BBox/MultiPoint/Point,ImageCollection.mosaic,Image.rename.Commits
Checks
devtools::document()rundevtools::test()passes, including new tests for the builder, point alignment, message escaping, project guard, recorded fixtures, cache formatlintr::lint_package(): no new lints in touched files; the object-usage linter still reports internal helpers it cannot resolveNEWS.mdupdated@examplesrunnable; commented-out code removedR CMD build)air format .not run; files formatted by hand to the existing styleNotes for review
were captured from the live service with the project redacted. The 403 and
429 fixtures are written from Google's documented error shapes and say so
in their files.
options(geefetch.rest_base)intests/testthat/setup.R, so fixture pathsin the tarball stay under 100 bytes. Recording uses the real host.
Either implement it or reword the README before CRAN submission.
against: the reader samples
CSIRO/SLGA/CLYand its siblings, but thepublic catalogue publishes one record for
CSIRO/SLGAand none for theimages inside it.
Linked issues
Closes #
Breaking changes?
No exported signature changed.
collect_gee_data()now honoursdepthandstaton the point route and returns NDVI rather than raw reflectance forthe two index datasets; both are documented behaviour that was not being
delivered. Disk caching is opt-in, so results persist across sessions only
after
gee_cache_disk(TRUE)or the matching option; an existing on-diskcache is rebuilt on first use under the new format.