Skip to content

cd_summary(): tell apart stations sharing a long_name and mixed-scale trends - #108

Merged
NewGraphEnvironment merged 11 commits into
mainfrom
98-cd-summary-stations-sharing-a-long-name
Sep 30, 2026
Merged

NewGraphEnvironment merged 11 commits into
mainfrom
98-cd-summary-stations-sharing-a-long-name

Conversation

@NewGraphEnvironment

@NewGraphEnvironment NewGraphEnvironment commented Sep 30, 2026 •

Copy link
Copy Markdown
Owner

Summary

cd_summary() keeps every row distinguishable (#98).

  • Stations sharing a long_name. A long_name carried by several variables gets the variable appended: Mean discharge (q_site1), Mean discharge (q_site2). The rule is the one cd_plot_comparison() has used since cd_plot_timeseries() and cd_plot_comparison(): use long_name/unit carried on the input #93, lifted into one internal helper, label_disambiguate() in R/cd_anomaly.R, which both call. Registered ERA5 variables are unaffected, because registry long_names are unique (pinned by a test).
  • Raw-value and anomaly trends in one table. bind_rows(cd_trend(x), cd_trend(ano)) gave rows differing by Unit at most, and for tmean not even that. When a table holds both scales, a Trend on column (Value/Anomaly) now follows Period. It uses the same rule as Unit: only trend_on == "value" is a raw-value trend, and a missing or NA value reads as Anomaly. A table on one scale keeps its shape, so both vignettes' tables are unchanged.

Behaviour changes for existing callers:

  • cd_summary() Parameter gains (variable) wherever several variables share a label.
  • cd_summary() gains a Trend on column only when the input mixes scales.
  • cd_plot_comparison() in the contrived case where a suffixed label meets another variable's own label (long_name = c("Q", "Q", "Q (a)")): the facets now read Q (a) (a), Q (b), Q (a) (c) instead of Q (a), Q (b), Q (a). Each variable already had its own facet, and now each also has its own label.
  • A variable name built to collide (a) (a beside a) gives labels that never settle, so cd_summary() and cd_plot_comparison() abort ("Rename the variables…") rather than print two variables under one name. cd_plot_comparison() plotted that input on main.

Review

Records are in planning/archive/2026-09-issue-98-summary-station-labels/.

  • Plan review: no blockers. It found a third source of look-alike rows, several trend_start values, now filed as cd_summary(): trends from several trend_start values differ only by Years #106.
  • /code-check, three rounds over the whole branch. The helper's repeat loop was twice bounded by reasoning, and wrong both times:
    • Round 1: the bound (number of variables) was too small when one variable carries different labels by period.
    • Round 2: a variable name built to collide (a) (a) never settles, so the helper aborts. That abort is now pinned by a test.
    • Round 3: narrowed an overclaim about which functions call the helper. cd_plot_timeseries() plots one variable and does not.
    • The loop ended by enumeration, not a reviewer's say-so. Every set of 1-4 (variable, label) pairs over a small alphabet, 66,711 sets, gave 0 aborts and 0 labels shared by two variables.
  • Mutation check: removing the station fix, the Trend on block, or the helper's abort each turns the new tests red.

Test plan

  • devtools::test(): 395 pass, 0 fail. The 6 warnings are the existing row-names warning in cd_plot_comparison().
  • devtools::document() regenerated cd_summary.Rd, cd_plot_comparison.Rd and cd_plot_timeseries.Rd.
  • pkgdown::check_pkgdown() fails on main already (DESCRIPTION URL lacks the github.io address). This branch adds no export.

Notes

Fixes #98

Relates to NewGraphEnvironment/sred-2025-2026#23

🤖 Generated with Claude Code

https://claude.ai/code/session_01GignjoKpZADXywTf6pAMgj

NewGraphEnvironment and others added 11 commits September 30, 2026 07:53
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GignjoKpZADXywTf6pAMgj
cd_plot_comparison()'s inline #93 rule moves to one helper in
R/cd_anomaly.R so cd_summary() can call the same rule. The helper
repeats until no label is shared by two variables, since a table has
no hidden facet key to fall back on.

Relates to #98

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GignjoKpZADXywTf6pAMgj
Two stations carrying one long_name printed as identical rows once
cd_summary() dropped variable. Parameter now goes through
label_disambiguate(), matching cd_plot_comparison()'s labels.

Relates to #98

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GignjoKpZADXywTf6pAMgj
…share

Plan review: variable names containing parentheses can make the bounded
loop end with a label still shared, silently. A post-loop check now
aborts. The plot collision test pins the new facet labels, and a
variable carrying different long_names by period is covered.

Relates to #98

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GignjoKpZADXywTf6pAMgj
…rends

bind_rows(cd_trend(x), cd_trend(ano)) gave rows differing by Unit at
most - for tmean not even that. A Trend on column (Value/Anomaly) now
follows Period when the table holds both scales; a single-scale table
keeps its shape. Same rule as Unit: only trend_on == "value" is a
raw-value trend.

Relates to #98

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GignjoKpZADXywTf6pAMgj
Relates to #98

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GignjoKpZADXywTf6pAMgj
Code-check round 1: one variable can carry a different label per period,
so a collision chain can outrun the variable count and abort on valid
input. Enumerated 66,711 small inputs at the new bound: no aborts, no
label shared by two variables. Also drops cd_summary() advice to bind
per-region trend tables before summarising, which would merge regions.

Relates to #98

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GignjoKpZADXywTf6pAMgj
Code-check round 2: a variable name built to collide ("a) (a" beside
"a") makes labels that never settle, so the comment's claim that the
pair count is the bound held only for the enumerated names. It aborts;
now pinned by a test and said in the comment.

Relates to #98

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GignjoKpZADXywTf6pAMgj
Code-check round 3: CLAUDE.md and the helper comment said every label
printer calls it, but cd_plot_timeseries() plots one caller-chosen
variable and does not. Claims narrowed; cd_plot_timeseries() docs say
its label is not suffixed and the station belongs in title.

Relates to #98

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GignjoKpZADXywTf6pAMgj
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GignjoKpZADXywTf6pAMgj
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GignjoKpZADXywTf6pAMgj
@NewGraphEnvironment
NewGraphEnvironment merged commit 8fc7ba0 into main Sep 30, 2026
1 check passed
@NewGraphEnvironment
NewGraphEnvironment deleted the 98-cd-summary-stations-sharing-a-long-name branch September 30, 2026 15:43
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.

cd_summary(): stations sharing a long_name give indistinguishable rows

1 participant