Skip to content

fix: support tsam_xarray 0.6.5 by using cluster_counts - #755

Merged
FBumann merged 1 commit into
ci/pin-ruff-versionfrom
chore/tsam-xarray-0.6.5
Jul 27, 2026
Merged

fix: support tsam_xarray 0.6.5 by using cluster_counts#755
FBumann merged 1 commit into
ci/pin-ruff-versionfrom
chore/tsam-xarray-0.6.5

Conversation

@FBumann

@FBumann FBumann commented Jul 27, 2026

Copy link
Copy Markdown
Member

Problem

tsam_xarray 0.6.5 renamed AggregationResult.cluster_weights to cluster_counts, keeping the old name as a deprecated property. Since our pytest config promotes warnings to errors (pyproject.toml:214), the resulting FutureWarning fails almost the entire clustering suite on any environment that resolves to 0.6.5 — and the full extra's current floor of >= 0.6.1, < 1 already permits it, so a fresh install hits this today.

tests/test_clustering on main:

tsam_xarray 0.6.1 0.6.5
main 286 passed 254 failed, 32 passed
this PR 286 passed

Every one of the 254 failures is the same FutureWarning: AggregationResult.cluster_weights is deprecated; use cluster_counts instead.

Change

  • transform_accessor.pycluster_weightscluster_counts in build_cluster_weights
  • pyproject.tomlfull: >= 0.6.1>= 0.6.5; dev: ==0.6.1==0.6.5

cluster_counts does not exist before 0.6.5, so the floor has to move with the rename rather than merely allowing the new version. The two changes are coupled and can't land separately.

Bonus: expand() gets ~3.6x faster

0.6.5 also vectorises unsegmented disaggregation, which speeds up expand() with no changes on our side. Measured on a 3-year, 40-cluster, 84-variable system, min of 10 runs:

tsam_xarray expand()
0.6.1 0.262 s
0.6.5 0.073 s

cluster() is unaffected (0.096 s vs 0.095 s, within noise). Users on the full extra already resolved to 0.6.5 and have this; the exact pin in dev was holding CI and development environments on the slow path.

Note for #748

This touches build_cluster_weights, which #748 (reapply #739) also rewrites. Whoever lands that should keep cluster_counts — the rename is independent of the dim-naming work and is required for 0.6.5 either way.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved cluster weighting metadata when building reduced flow systems.
  • Chores

    • Updated the optional full and development dependency requirements for improved compatibility.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The reduced flow-system builder now sources cluster weights from aggregation cluster counts. Optional and development dependency requirements are updated to tsam_xarray 0.6.5.

Changes

Reduced clustering weights

Layer / File(s) Summary
Clustering dependency constraints
pyproject.toml
The full optional dependency requires tsam_xarray ≥ 0.6.5, while the development dependency pins 0.6.5.
Reduced cluster weight construction
flixopt/transform_accessor.py
build_cluster_weights() derives cluster_weight from agg_result.cluster_counts.

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

Possibly related PRs

  • flixOpt/flixopt#593: Introduced and refactored the reduced clustering builder and its weight logic.
  • flixOpt/flixopt#654: Rewired reduced clustering construction around tsam_xarray aggregation outputs.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is detailed, but it does not follow the required template sections for type, related issues, testing, or checklist. Add the template sections: Type of Change, Related Issues, Testing, and Checklist, and fill them out explicitly.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly summarizes the main change: updating flixopt for tsam_xarray 0.6.5 and switching to cluster_counts.
✨ 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 chore/tsam-xarray-0.6.5

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

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

tsam_xarray 0.6.5 renamed AggregationResult.cluster_weights to
cluster_counts, keeping the old name as a deprecated property. Because
the test suite promotes warnings to errors, the resulting FutureWarning
fails 254 of 286 clustering tests on any environment that resolves to
0.6.5 -- which the current `full` floor of >= 0.6.1 already permits.

cluster_counts does not exist before 0.6.5, so the floor moves with the
rename rather than merely allowing the new version.

0.6.5 also vectorises unsegmented disaggregation, which makes expand()
~3.6x faster with no changes on our side: 0.262s -> 0.073s on a 3-year,
40-cluster, 84-variable system (min of 10 runs).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@FBumann
FBumann force-pushed the chore/tsam-xarray-0.6.5 branch from 7220a64 to 0bdedf8 Compare July 27, 2026 12:48
@FBumann
FBumann changed the base branch from main to ci/pin-ruff-version July 27, 2026 12:48
@FBumann
FBumann merged commit ea61037 into ci/pin-ruff-version Jul 27, 2026
12 checks passed
FBumann added a commit that referenced this pull request Jul 27, 2026
tsam_xarray 0.6.5 renamed AggregationResult.cluster_weights to
cluster_counts, keeping the old name as a deprecated property. The `full`
extra allows >= 0.6.1, so a fresh install resolves to 0.6.5 and every
cluster() call emits a FutureWarning -- and fails outright under -W error,
which is how the test suite runs: 254 of 286 clustering tests failed on
0.6.5 before this change.

cluster_counts does not exist before 0.6.5, so the floor moves with the
rename rather than merely allowing the new version.

This shipped in neither 8.0.0 nor its predecessor: #755 carried the same
change but was retargeted onto an intermediate branch that merged into main
first, so it landed on that branch instead of main.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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