feat: validate extremes options against cluster_on

[FBumann]

What

Follow-up to #89. Adds up-front validation for the two ways an ExtremeConfig silently misbehaves when combined with cluster_on.

Under cluster_on, the clustering is built on the active subset and transferred to the carried columns via ClusteringResult.apply(). Two extremes settings don't survive that transfer, and today both fail unhelpfully:

Combination	Before	After
method="replace"	silently degrades — warning only, result falls back to the medoid representative	clear ValueError, steers to weights
extreme references a cluster_on-excluded coord	tsam raises misleading Extreme period columns not found in data (the column is in the DataArray)	clear ValueError naming the real cause
extreme on an active/clustered-on column (append/new_cluster)	works	still works (guarded by a positive test)

Why

• replace: apply() can't reproduce the hybrid replace representative (some columns from the medoid, some from the extreme period), so it quietly returns the medoid. The error points users to weights instead: a low weight de-emphasises a column without excluding it from the single aggregation pass, so replace stays reproducible. (The very weights-clamp that makes weights=0 a footgun for exclusion is what makes it the right tool here.)
• excluded column: extreme periods can only be identified on columns that drive the clustering. Referencing a carried-through coord can't work, so say so plainly.

How

Both checks live in one _validate_extremes_with_cluster_on(), called right after cluster_on normalization — so they fail before any aggregation runs. Only active when cluster_on actually restricts.

Tests

• test_replace_extremes_rejected — rejection + message
• test_extremes_on_excluded_column_rejected — clear cause, not tsam's cryptic error
• test_append_extremes_allowed — locks in that we do not over-reject legitimate append

Full suite green, mypy + ruff clean. Docstring updated with both restrictions.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved validation when combining clustering options with extreme-value handling, preventing unsupported configurations and returning clearer error messages.
  • Allowed compatible extreme-value settings to continue working when clustering transfer is enabled.

• Documentation

  • Updated usage guidance to clarify which extreme-value settings are incompatible with clustering options and suggest a safer alternative.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds validation in aggregate() to reject incompatible extremes and cluster_on combinations via a new _validate_extremes_with_cluster_on() helper, updates the cluster_on docstring, and adds tests covering rejected and allowed configurations.

Changes

Extremes/cluster_on validation

Layer / File(s)	Summary
Validator implementation and wiring src/tsam_xarray/_core.py	Adds _validate_extremes_with_cluster_on() which rejects extremes.method="replace" combined with cluster_on and rejects ExtremeConfig coords excluded by cluster_on; wires the call into aggregate() after normalizing cluster_on; updates the cluster_on docstring to document the incompatibility and recommend using weights instead.
Validator tests test/test_aggregate.py	Adds TestClusterOn tests verifying rejection of replace extremes and excluded-coord extremes, and confirming append extremes remain allowed with cluster_on.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• FBumann/tsam_xarray#9: Both PRs modify aggregate()'s handling of tsam_kwargs, with this PR adding extremes/cluster_on compatibility checks on top of the aggregate wrapper introduced there.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding validation for extremes options when used with cluster_on.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/cluster-on-extremes-validation

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/tsam_xarray/_core.py (1)

184-200: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add Args: and Raises: sections to the docstring.

This validator's sole purpose is to raise, yet the docstring documents neither its parameters nor the ValueError it raises. Add Google-style Args: and Raises: sections.

As per coding guidelines: "Use Google-style docstrings with Args:, Returns:, Raises:, Attributes: sections".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/tsam_xarray/_core.py` around lines 184 - 200, The docstring on the
validator in the `_core.py` cluster_on validation helper currently explains the
behavior but does not follow the required Google-style structure. Update that
docstring to include an `Args:` section for the validator’s inputs and a
`Raises:` section documenting the `ValueError` it can emit, keeping the existing
explanation tied to the cluster_on/ExtremeConfig checks and the
`method="replace"` handling.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/tsam_xarray/_core.py`:
- Around line 184-200: The docstring on the validator in the `_core.py`
cluster_on validation helper currently explains the behavior but does not follow
the required Google-style structure. Update that docstring to include an `Args:`
section for the validator’s inputs and a `Raises:` section documenting the
`ValueError` it can emit, keeping the existing explanation tied to the
cluster_on/ExtremeConfig checks and the `method="replace"` handling.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4deddda6-4048-438b-a328-257165a93455

📥 Commits

Reviewing files that changed from the base of the PR and between ddff73b and f9c6119.

📒 Files selected for processing (2)

• src/tsam_xarray/_core.py
• test/test_aggregate.py