Skip to content

Add a vignette, and fix the row reordering it exposed - #29

Merged
chross22 merged 2 commits into
masterfrom
vignette
Aug 7, 2026
Merged

Add a vignette, and fix the row reordering it exposed#29
chross22 merged 2 commits into
masterfrom
vignette

Conversation

@chross22

@chross22 chross22 commented Aug 7, 2026

Copy link
Copy Markdown
Owner

The README explains but never demonstrates — every example in it is eval = FALSE, because they need Copernicus credentials. The vignette runs.

Its covariates are synthetic (a Gulf of Maine grid with a seasonal cycle and winter cloud gaps), so every number and figure is computed at build time, while the calls are the ones you would make against a real fetch.

That is the part the README cannot do:

  • coverage dropping to 0.17 in January and back to 0.92 in June is a sentence in the README and a figure here
  • min_coverage reporting 1 cell rather than 39 is an argument there and an output here
  • downscale_grid(method = "nearest") inventing no values is a claim there and TRUE here

Writing it found a bug

matchData() processes rows a period at a time, so a table whose periods were interleaved came back grouped by period rather than in the order it arrived:

input  id: 1 2 3 4
output id: 1 3 2 4

Anyone aligning the result against the input by position — cbind(), or assigning a column straight across — would have got silently mismatched rows.

Worse: I had documented the opposite this afternoon, in both the roxygen and the vignette draft, having assumed it rather than checked. The order is now restored before returning, which makes the claim true rather than retracting it.

A test pins it, and checks the covariate follows its own row rather than merely the row count — the failure mode where row counts match but values are shuffled.

Also

  • Vignette infrastructure in DESCRIPTION (knitr, rmarkdown, VignetteBuilder).
  • figure/ — knitr scratch output from running knit() directly rather than through R CMD build — is gitignored after briefly being committed.

Verification

R CMD checkStatus: OK, including checking re-building of vignette outputs ... OK. Full suite green. Installed into both R 4.6.1 and 4.3.2.

Note: pandoc is not on this machine's PATH; the build used the copy RStudio bundles. CI has its own.

🤖 Generated with Claude Code

chross22 and others added 2 commits August 7, 2026 16:37
The README explains but never demonstrates: every example in it is
eval = FALSE, because they need Copernicus credentials. The vignette
runs. Its covariates are synthetic - a Gulf of Maine grid with a
seasonal cycle and winter cloud gaps - so every number and figure in it
is computed at build time, while the calls are the ones you would make
against a real fetch.

That is the part the README cannot do. Coverage dropping to 0.17 in
January and back to 0.92 in June is a sentence in the README and a
figure here; min_coverage reporting 1 cell rather than 39 is an
argument there and an output here.

Writing it found a bug. matchData() processes rows a period at a time,
so a table whose periods were interleaved came back grouped by period
rather than in the order it arrived: 1 2 3 4 in, 1 3 2 4 out. Anyone
aligning the result against the input by position - cbind(), or
assigning a column straight across - would have got silently mismatched
rows.

Worse, I had documented the opposite this afternoon, in both the roxygen
and the vignette draft, having assumed it rather than checked. The order
is now restored before returning, which makes the claim true rather than
retracting it, and a test pins it: the covariate has to follow its own
row, not merely the row count.

Vignette infrastructure added to DESCRIPTION (knitr, rmarkdown,
VignetteBuilder), and built HTML is gitignored.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
figure/ is what knitr::knit() writes when run directly on the vignette
rather than through R CMD build, which puts its figures inside the
package. Three PNGs of synthetic data, regenerated on every build and
belonging in neither the repository nor the tarball.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chross22
chross22 merged commit c13bda0 into master Aug 7, 2026
5 checks passed
@chross22
chross22 deleted the vignette branch August 7, 2026 20:45
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.

1 participant