Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ The historical `cd_fetch()` / `cd_derive()` R-side producer functions still ship

**Key design decision:** Raw climate values on STAC (not pre-computed anomalies). All baseline/anomaly/trend computation consumer-side for maximum flexibility over reference periods and comparison windows.

**The consumer chain is not ERA5-only.** `cd_baseline()` → `cd_anomaly()` → `cd_trend()` → `cd_summary()` / `cd_compare()` take any series in the long format (`variable`, `period`, `year`, `value`, optional `anomaly_type`/`unit`/`long_name`) — other packages (wet's streamflow) target it. The contract lives in `?cd_anomaly`; the rules live in three helpers in `R/cd_anomaly.R` (`series_check`, `meta_resolve`, `meta_check`). Call them from any new consumer function rather than reading `cd_variables()` directly — per-function copies are what three review rounds kept finding broken (#92).
**The consumer chain is not ERA5-only.** `cd_baseline()` → `cd_anomaly()` → `cd_trend()` → `cd_summary()` / `cd_compare()` take any series in the long format (`variable`, `period`, `year`, `value`, optional `anomaly_type`/`unit`/`long_name`) — other packages (wet's streamflow) target it. The contract lives in `?cd_anomaly`; the rules live in four helpers in `R/cd_anomaly.R` (`series_check`, `meta_resolve`, `meta_check`, `label_disambiguate`). Call them from any new consumer function rather than reading `cd_variables()` directly — per-function copies are what three review rounds kept finding broken (#92). Anything that labels several variables side by side (a table, facets) passes the labels through `label_disambiguate()`, which appends ` (variable)` to a `long_name` several stations share; `cd_summary()` and `cd_plot_comparison()` both do (#98). `cd_plot_timeseries()` does not — it plots one variable the caller chose, so the station belongs in its `title`.

**A trend table can mix scales.** Since #97 every `cd_trend()` row carries `trend_on` (`"value"` / `"anomaly"`), and `bind_rows(cd_trend(x), cd_trend(ano))` is a legal input. Any consumer of a trend table reads it with `col_or_na(trend, "trend_on")`: a missing or `NA` `trend_on` (hand-built tables, tables saved before 0.5.2) must not error. `cd_summary()` reads a missing `trend_on` as anomaly; `cd_plot_timeseries()` draws it on either scale (#103).

Expand Down
35 changes: 35 additions & 0 deletions R/cd_anomaly.R
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,41 @@ meta_resolve <- function(x, raw = FALSE) {
)
}

#' Make labels tell variables apart: a label shared by several variables (one
#' long_name on many stations) gets ` (variable)` appended. Repeated because a
#' suffixed label can meet another variable's own label (`c("Q", "Q", "Q (a)")`);
#' each pass lengthens only the labels still shared. A chain of such collisions
#' can be as long as the distinct (variable, label) pairs, not the variables —
#' one variable may carry a different label per period — so that is the bound;
#' it held for every small input enumerated in #98. A variable name built to
#' collide (`"a) (a"` beside `"a"`) can make labels that never settle, so a label
#' still shared after `max_passes` aborts rather than printing two variables
#' under one name. The one place the rule lives; every consumer that labels
#' several variables side by side (a table, facets) calls it.
#' @noRd
label_disambiguate <- function(variable, label,
max_passes = nrow(unique(data.frame(v = as.character(variable), l = label)))) {
variable <- as.character(variable)
shared_find <- function(label) {
lab <- unique(data.frame(variable = variable, label = label))
label %in% lab$label[duplicated(lab$label)]
}
for (i in seq_len(max_passes)) {
shared <- shared_find(label)
if (!any(shared)) return(label)
label[shared] <- paste0(label[shared], " (", variable[shared], ")")
}
shared <- shared_find(label)
if (any(shared)) {
rlang::abort(paste0(
"Could not give each variable its own label; still shared: ",
paste(utils::head(unique(label[shared]), 5), collapse = ", "),
". Rename the variables or give them distinct `long_name`s."
))
}
label
}

#' Abort unless each variable/period carries one value of each metadata
#' column present in `x` (NA counts as a value). `x` must be ungrouped.
#' @noRd
Expand Down
13 changes: 7 additions & 6 deletions R/cd_plot_comparison.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
#' `period`, `mean_a`, `mean_b`, `difference`, optionally
#' `long_name`. Facets are labelled by `long_name` where present and
#' not `NA`, otherwise by [cd_variables()], otherwise by `variable`;
#' a label shared by several variables gets the variable name appended.
#' Each variable gets its own facet whatever the labels.
#' a label shared by several variables gets the variable name appended,
#' as in [cd_summary()]. Each variable gets its own facet whatever the
#' labels.
#' @param title Optional plot title.
#' @param labels Named character vector of length 2 for window labels.
#' Default `c(a = "Recent", b = "Historical")`.
Expand All @@ -34,10 +35,10 @@ cd_plot_comparison <- function(x,
# Facet labels: carried long_name, then cd_variables(), then the name.
# A label shared by several variables (one long_name, many stations)
# carries the variable name too, so their facets read differently.
x$param <- dplyr::coalesce(meta_resolve(x)$long_name, as.character(x$variable))
lab <- unique(data.frame(variable = as.character(x$variable), param = x$param))
shared <- x$param %in% lab$param[duplicated(lab$param)]
x$param[shared] <- paste0(x$param[shared], " (", x$variable[shared], ")")
x$param <- label_disambiguate(
x$variable,
dplyr::coalesce(meta_resolve(x)$long_name, as.character(x$variable))
)
# Facet on variable and label together, never the label alone, so no
# label, however it collides, can put two variables in one facet.
# Levels in label order, as faceting by label gave.
Expand Down
4 changes: 3 additions & 1 deletion R/cd_plot_timeseries.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
#' plotted column's name. `unit` is the
#' anomaly's unit, so on raw `value` input it is shown only where the
#' anomaly type is `absolute` or `pct_point_diff`, whose anomaly unit is
#' the unit of the values.
#' the unit of the values. Unlike [cd_summary()], a `long_name` shared by
#' several variables is not suffixed with the variable: the plot shows one
#' variable, so name a station in `title`.
#'
#' @param x A tibble from [cd_anomaly()] with columns `variable`,
#' `period`, `year`, `anomaly`, optionally `anomaly_type`, `unit` and
Expand Down
29 changes: 27 additions & 2 deletions R/cd_summary.R
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,24 @@
#' series is `"%"`, which does not describe a slope in mm — so it agrees
#' with the axis label of [cd_plot_timeseries()] on the same series.
#'
#' Rows are kept distinguishable. A `long_name` shared by several variables
#' (one label on many stations) gets the variable name appended, as in
#' [cd_plot_comparison()]: `"Mean discharge (q_site1)"`. A table holding both
#' raw-value and anomaly trends, such as
#' `dplyr::bind_rows(cd_trend(x), cd_trend(ano))`, gains a `Trend on` column
#' (`"Value"` or `"Anomaly"`; a missing or `NA` `trend_on` reads as
#' `"Anomaly"`). A table on one scale has no such column. Both are decided
#' within one call, so summaries bound together (one per region, each with
#' its `region_name`) can differ in suffixes, and a `Trend on` column present
#' in only some of them is `NA` for the rest.
#'
#' @param trend A tibble from [cd_trend()].
#' @param region_name Optional character label for the AOI. If provided,
#' adds a `Region` column.
#'
#' @return A tibble with columns `Parameter`, `Period`, `Slope`, `Years`,
#' `Total Change`, `Unit`, `p-value`, and optionally `Region`.
#' `Total Change`, `Unit`, `p-value`, and optionally `Region`. When `trend`
#' mixes raw-value and anomaly trends, a `Trend on` column follows `Period`.
#'
#' @examples
#' catalog <- cd_catalog(
Expand All @@ -43,7 +55,10 @@
cd_summary <- function(trend, region_name = NULL) {
trend <- dplyr::ungroup(trend)
meta <- meta_resolve(trend, raw = col_or_na(trend, "trend_on") %in% "value")
labels_param <- dplyr::coalesce(meta$long_name, as.character(trend$variable))
labels_param <- label_disambiguate(
trend$variable,
dplyr::coalesce(meta$long_name, as.character(trend$variable))
)
labels_unit <- meta$unit

out <- trend |>
Expand All @@ -58,6 +73,16 @@ cd_summary <- function(trend, region_name = NULL) {
) |>
dplyr::select("Parameter", "Period", "Slope", "Years", "Total Change", "Unit", "p-value")

# A table mixing raw-value and anomaly trends of one series would otherwise
# give rows that differ by Unit at most. Same rule as Unit: only "value" is
# a raw-value trend; anything else, NA included, reads as anomaly.
on_value <- col_or_na(trend, "trend_on") %in% "value"
if (length(unique(on_value)) > 1) {
out <- tibble::add_column(
out, `Trend on` = dplyr::if_else(on_value, "Value", "Anomaly"), .after = "Period"
)
}

if (!is.null(region_name)) {
out$Region <- region_name
}
Expand Down
5 changes: 3 additions & 2 deletions man/cd_plot_comparison.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion man/cd_plot_timeseries.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 13 additions & 1 deletion man/cd_summary.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions planning/archive/2026-09-issue-98-summary-station-labels/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
## Outcome

`cd_summary()` keeps rows distinguishable (#98). A `long_name` shared by several variables (one label on many stations) gets ` (variable)` appended, by a rule lifted out of `cd_plot_comparison()` into one internal helper, `label_disambiguate()` in `R/cd_anomaly.R`, which both now call. A table that mixes raw-value and anomaly trends gains a `Trend on` column (`Value`/`Anomaly`, the same `%in% "value"` rule as `Unit`); single-scale tables keep their shape, so the vignettes are unchanged. The user chose the conditional column at the plan gate over always adding it or suffixing `Parameter`. What was learned: the helper's repeat loop was bounded by reasoning three times and wrong twice — the plan review caught a silent residual collision, code-check round 1 a bound (variables) too small for one variable carrying different labels by period, round 2 a variable-name shape (`a) (a`) that never settles and so aborts. The shared mechanism, named in round 3, is assuming `variable` makes a printed thing unique; the loop ended on enumeration, not on a reviewer's say-so. A third source of look-alike rows (several `trend_start` values) is filed as #106.

## Measurement

Exhaustive enumeration of `label_disambiguate()` (`label_disambiguate_enum.R`): every set of 1–4 distinct (variable, label) pairs over variables `a`, `b`, `a) (b` and 12 chained-suffix labels — 66,711 sets, 0 aborts, 0 labels shared by two variables at the pair-count bound. At the first bound (`n_distinct(variable)`) the round-1 reviewer's 40,000-draw fuzz aborted 11 times; at the pair count, 0. 969 of the 66,711 sets merge two labels of one variable across periods (accepted: rows differ by `Period`, as on main). Round 2 and 3 stress runs (50,000 random inputs) never exceeded the bound. Final suite: 395 pass, 0 fail.

## Evidence

`review-plan.md`, `review-round*.md`, `label_disambiguate_enum.R` in this directory.

Closed by: PR (see `gh pr list --search 98`)
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Findings — cd_summary(): stations sharing a long_name give indistinguishable rows (#98)

## Issue context

**If we do it:** a `cd_summary()` table over several stations that share one `long_name` says which row is which station. **If we never do:** two rows read `Mean discharge / Annual` with different slopes and nothing to tell them apart — while `cd_plot_comparison()` beside the table labels the same series `Mean discharge (q_site1)`.

## Problem

`series_check()` tells callers that series from several sites need distinct `variable` names. The natural next step for streamflow is one `long_name` ("Mean discharge") on every station. `cd_summary()` (`R/cd_summary.R`) replaces `variable` with `Parameter = long_name` and drops `variable`, so those rows become indistinguishable. Measured in #93's review with `q_site1` / `q_site2`.

#93 handled the same case in `cd_plot_comparison()`: a label shared by several variables gets ` (variable)` appended, and facets key on variable + label so nothing can merge.

## Proposed Solution

- In `cd_summary()`, append ` (variable)` to a `Parameter` shared by more than one variable — the same rule as `cd_plot_comparison()`, ideally lifted into one internal helper both call.
- Test: two stations, one `long_name` → two distinct `Parameter` values; registered ERA5 variables unchanged (registry long_names are unique).

A second way to get indistinguishable rows: since #97 a trend table can hold both a raw-value and an anomaly trend of one series (`bind_rows(cd_trend(x), cd_trend(ano))`, told apart by `trend_on`). `cd_summary()` drops `trend_on`, so those rows differ only by `Unit`, and for an `absolute` variable such as tmean not even by that. Whatever disambiguates stations here should also cover `trend_on`, or `cd_summary()` should carry it.

Found by `/code-check` on #93; the `trend_on` case by the plan review for #97.

## Plan-gate decision (2026-09-29)

`trend_on` disambiguation: a conditional `Trend on` column ("Value"/"Anomaly"), added only
when the table holds both scales — the `Region` precedent. Chosen over always adding the
column (changes every existing table, both vignettes' kables) and over suffixing `Parameter`
(scale ends up in a text label rather than a filterable field).

## Exploration

- `cd_plot_comparison()` (`R/cd_plot_comparison.R:34-39`) has the #93 rule inline: a label
shared by several variables gets ` (variable)` appended, one pass. Its residual collision
(`long_name = c("Q", "Q", "Q (a)")`) is harmless there because facets key on
variable + label. A table has no hidden key, so the lifted helper repeats until labels are
distinct per variable.
- Shared helpers live in `R/cd_anomaly.R` (`col_or_na`, `series_check`, `meta_resolve`,
`meta_check`); CLAUDE.md tells new consumers to call them.
- `cd_summary()` already reads a missing/`NA` `trend_on` as anomaly (`meta_resolve(raw = col_or_na(...) %in% "value")`).

## Errors Encountered

| Error | Resolution |
|-------|------------|

## label_disambiguate() pass bound — enumeration (2026-09-30)

Code-check round 1 showed the first bound (`n_distinct(variable)`) was reasoned, not measured:
`label_disambiguate(c("a","e","e","e"), c("Q","Q","Q (a)","Q (a) (a)"))` aborted after two
passes and settles at three, because one variable can carry a different label per period. The
bound is now the number of distinct (variable, label) pairs.

Enumeration (`label_disambiguate_enum.R`): every set of 1–4 distinct pairs over variables
`a`, `b`, `a) (b` and 12 labels built by chaining their suffixes to depth 2 — 66,711 sets.
**0 aborts, 0 labels shared by two variables.** 969 sets merge two labels of *one* variable
(`a` carrying `Q` and `Q (a)` in different periods both print `Q (a)`); those rows still differ
by `Period`, and the one-pass rule on main does the same. Accepted.

Bound of the enumeration: its names were `a`, `b`, `a) (b`. Round 2 found a shape outside it —
variables `a` and `a) (a`, with `a) (a` labelled `Q` and `Q (a)` — that never settles at any
bound (tried to 50). It aborts with "Rename the variables…"; `cd_plot_comparison()` plotted it on
main. Needs a name built to collide; accepted, pinned by a test, stated in the helper comment.
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
devtools::load_all(quiet = TRUE)
vs <- c("a", "b", "a) (b")
sfx <- function(l) as.vector(outer(l, vs, function(x, v) paste0(x, " (", v, ")")))
labs <- unique(c("Q", sfx("Q"), sfx(sfx("Q"))))
pairs <- expand.grid(v = vs, l = labs, stringsAsFactors = FALSE)
n_abort <- 0; n_bad <- 0; n <- 0; max_used <- 0
for (k in 1:4) {
cmb <- utils::combn(nrow(pairs), k)
for (j in seq_len(ncol(cmb))) {
p <- pairs[cmb[, j], ]
n <- n + 1
out <- tryCatch(label_disambiguate(p$v, p$l), error = function(e) NULL)
if (is.null(out)) { n_abort <- n_abort + 1; next }
u <- unique(data.frame(v = p$v, l = out))
if (anyDuplicated(u$l) && any(duplicated(u$l) & !duplicated(u[c("l")]) | TRUE) && any(tapply(u$v, u$l, function(z) length(unique(z))) > 1)) { n_bad <- n_bad + 1; cat("SHARED\n") }
# rows of one variable with one label keep one label (no split)
if (nrow(u) != nrow(unique(p))) { n_merge <- get0("n_merge", ifnotfound = 0) + 1; if (n_merge == 1) print(cbind(p, out)) }
}
}
cat("merged-within-variable:", get0("n_merge", ifnotfound = 0), "\n"); cat("sets:", n, " labels:", length(labs), " aborts:", n_abort, " violations:", n_bad, "\n")
Loading
Loading