Skip to content

Review round 1: tests in R CMD check, API errors, safer rmGeoIdentity - #14

Merged
kauedesousa merged 1 commit into
masterfrom
review/round-1
Oct 31, 2026
Merged

kauedesousa merged 1 commit into
masterfrom
review/round-1

Conversation

@kauedesousa

@kauedesousa kauedesousa commented Oct 31, 2026 •

Copy link
Copy Markdown
Member

First review round of ClimMobTools, same process as gosset.

Tests never ran. .Rbuildignore excluded ^tests$, so R CMD check skipped them, and they had gone stale: test-seed_need.R tests a function that no longer exists, and test-randomise.R used the old arguments nobservers/nitems. Now:

  • design-quality tests for randomise(): distinct items per package, equal item counts, balance across positions A/B/C, connected comparison network, availability limits, reproducibility with set.seed();
  • tests for rmGeoIdentity(): displacement within dist, NAs kept, no output, RNG untouched;
  • API tests read the key from CLIMMOB_KEY (skip_without_key() helper) instead of a key written in the test file. Add a CLIMMOB_KEY repository secret to run them in CI.

Code changes

  • getDataCM(), getProjectProgress(): request body built with jsonlite::toJSON() (the pasted string broke with quotes in project codes); clear error on HTTP 401/403/404/5xx instead of a JSON parse error.
  • rmGeoIdentity(): no longer prints the seed. The displacement does not depend on the original coordinates, so with the seed anyone can reproduce the offsets and recover exact locations; the docs now say so and the examples no longer use a seed. A given seed no longer changes the caller's RNG. sf is called by namespace, so it no longer has to be attached.

New randomise() algorithm (Jacob van Etten)

  • Sequentially balanced incomplete block design: every prefix of the design stays close to balanced, so it holds up when only the first packages are distributed or returned. Blocks are chosen by replication schedule, then exact A-optimality gain (rank-k update of the Laplacian inverse), with availability as a hard limit and sequential position balance. New arguments ridge, polish, schedule; old arguments such as comp are ignored with a warning. Designs differ from earlier versions for the same seed.
  • Base R only, so lpSolve, Matrix, RSpectra and methods are removed from Imports (now httr, jsonlite, stats, utils).
  • Extra tests: balanced prefixes, proportions, ncomp = 2, ignored arguments. The old diagnostic script (needs gosset) moved from tests/ to dev/.

Infrastructure: R CMD check workflow (Linux, macOS, Windows) with failures shown as annotations; AGENTS.md; NEWS.

Not changed, for discussion

  • An API key is written in the examples of getDataCM() and getSurvey(), the Overview vignette and the pkgdown site. If it is a public demo key that is fine; if not, it should be revoked on the server, since it is in the git history.
  • The request URLs contain Body={}&Apikey={} and httr appends the real Body and Apikey, so each parameter is sent twice. It works if the server reads the last value; worth cleaning with a server-side check.
  • The API key travels as a URL query parameter, so it can end up in server and proxy logs. A header would be safer, but that is a server change.
  • rankTricot() duplicates gosset::rank_tricot(); fixes now land in gosset only.
  • rmGeoIdentity() samples uniformly in the buffer, so a point can land very close to the original. Displacement with a minimum distance (as in DHS) would be stronger.

https://claude.ai/code/session_01LpE9sTbGPftSMogW6KpkrT


Generated by Claude Code

…rmGeoIdentity

- tests were excluded by .Rbuildignore; outdated tests replaced with
  design-quality tests for randomise() and tests for rmGeoIdentity()
- API tests read CLIMMOB_KEY from the environment
- request bodies built with jsonlite; clear errors on HTTP error status
- rmGeoIdentity(): seed no longer printed, caller's RNG restored, sf
  called by namespace, documentation warns that a known seed reveals the
  original locations
- R CMD check workflow, AGENTS.md, NEWS

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LpE9sTbGPftSMogW6KpkrT
@kauedesousa
kauedesousa marked this pull request as ready for review October 31, 2026 13:51
@kauedesousa
kauedesousa merged commit 5ee2b94 into master Oct 31, 2026
0 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants