From 633a59e2a6c12239a212a6e2f79c1c0e9a1035bb Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 20 Apr 2026 10:45:11 +0200 Subject: [PATCH 1/4] refactor: make Comparison follow dict/Mapping protocol Iteration now yields case names instead of (name, FlowSystem) pairs, matching Python's dict/Mapping convention. Added keys(), values(), and items() methods for explicit access. Users iterating pairs should switch to `for name, fs in comp.items():`. Co-Authored-By: Claude Opus 4.7 (1M context) --- flixopt/comparison.py | 24 ++++++++++++++++++++---- tests/test_comparison.py | 22 ++++++++++++++++++---- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/flixopt/comparison.py b/flixopt/comparison.py index 7e3e983e1..f1f6b1505 100644 --- a/flixopt/comparison.py +++ b/flixopt/comparison.py @@ -19,7 +19,7 @@ ) if TYPE_CHECKING: - from collections.abc import Iterator + from collections.abc import ItemsView, Iterator, KeysView, ValuesView from .flow_system import FlowSystem @@ -224,14 +224,30 @@ def __getitem__(self, key: int | str) -> FlowSystem: return self._systems[idx] raise KeyError(f"Case '{key}' not found. Available: {self._names}") - def __iter__(self) -> Iterator[tuple[str, FlowSystem]]: - """Iterate over (name, FlowSystem) pairs.""" - yield from zip(self._names, self._systems, strict=True) + def __iter__(self) -> Iterator[str]: + """Iterate over case names, matching the ``dict`` / ``Mapping`` protocol. + + Use :meth:`items` for ``(name, FlowSystem)`` pairs or :meth:`values` + for FlowSystems. + """ + return iter(self._names) def __contains__(self, key: str) -> bool: """Check if a case name exists.""" return key in self._names + def keys(self) -> KeysView[str]: + """Return a view of case names, like :meth:`dict.keys`.""" + return self.flow_systems.keys() + + def values(self) -> ValuesView[FlowSystem]: + """Return a view of FlowSystems, like :meth:`dict.values`.""" + return self.flow_systems.values() + + def items(self) -> ItemsView[str, FlowSystem]: + """Return a view of ``(name, FlowSystem)`` pairs, like :meth:`dict.items`.""" + return self.flow_systems.items() + @property def flow_systems(self) -> dict[str, FlowSystem]: """Access underlying FlowSystems as a dict mapping name → FlowSystem.""" diff --git a/tests/test_comparison.py b/tests/test_comparison.py index 7f7e7093e..f470e8760 100644 --- a/tests/test_comparison.py +++ b/tests/test_comparison.py @@ -212,11 +212,25 @@ def test_getitem_invalid_index_raises(self, optimized_base, optimized_with_chp): with pytest.raises(IndexError): _ = comp[99] - def test_iter(self, optimized_base, optimized_with_chp): - """Iteration yields (name, FlowSystem) pairs.""" + def test_iter_yields_names(self, optimized_base, optimized_with_chp): + """Iteration yields case names, matching the dict/Mapping protocol.""" comp = fx.Comparison([optimized_base, optimized_with_chp]) - items = list(comp) - assert items == [('Base', optimized_base), ('WithCHP', optimized_with_chp)] + assert list(comp) == ['Base', 'WithCHP'] + + def test_keys(self, optimized_base, optimized_with_chp): + """keys() returns case names.""" + comp = fx.Comparison([optimized_base, optimized_with_chp]) + assert list(comp.keys()) == ['Base', 'WithCHP'] + + def test_values(self, optimized_base, optimized_with_chp): + """values() returns FlowSystems.""" + comp = fx.Comparison([optimized_base, optimized_with_chp]) + assert list(comp.values()) == [optimized_base, optimized_with_chp] + + def test_items(self, optimized_base, optimized_with_chp): + """items() returns (name, FlowSystem) pairs without warning.""" + comp = fx.Comparison([optimized_base, optimized_with_chp]) + assert list(comp.items()) == [('Base', optimized_base), ('WithCHP', optimized_with_chp)] def test_contains(self, optimized_base, optimized_with_chp): """'in' operator checks for case name.""" From 6c5ec8b2b2053c5c19c0c78e9aae0d7701927915 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 20 Apr 2026 11:32:32 +0200 Subject: [PATCH 2/4] ci: pin xarray <2026.3 in dev deps for linopy compatibility linopy 0.6.x breaks with xarray 2026.3+ (TypeError on Dataset constructor). Keep the release dependency range unchanged; only pin for dev/CI until linopy ships a fix. Co-Authored-By: Claude Opus 4.7 (1M context) --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 642ad5765..51191d099 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -77,6 +77,7 @@ full = [ # Development tools and testing dev = [ + "xarray<2026.3", # TODO: drop once linopy ships xarray 2026.3+ compat fix "tsam==3.3.0", # Time series aggregation for clustering "pytest==9.0.3", "pytest-xdist==3.8.0", From 18ec614b9156ec8b150a88a6570b75ffc90e63bd Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 20 Apr 2026 12:38:22 +0200 Subject: [PATCH 3/4] feat: validate flow_systems input types in Comparison MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raise TypeError if flow_systems is not a list, or if any item is not a FlowSystem instance — replaces the cryptic AttributeError that would otherwise surface later when attributes like .name or .solution are accessed. Co-Authored-By: Claude Opus 4.7 (1M context) --- flixopt/comparison.py | 11 ++++++++++- tests/test_comparison.py | 10 ++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/flixopt/comparison.py b/flixopt/comparison.py index f1f6b1505..d35d5a49b 100644 --- a/flixopt/comparison.py +++ b/flixopt/comparison.py @@ -158,10 +158,19 @@ class Comparison: """ def __init__(self, flow_systems: list[FlowSystem], names: list[str] | None = None) -> None: + from .flow_system import FlowSystem + + if not isinstance(flow_systems, list): + raise TypeError(f'flow_systems must be a list, got {type(flow_systems).__name__}') + if len(flow_systems) < 2: raise ValueError('Comparison requires at least 2 FlowSystems') - self._systems = flow_systems + non_fs = [(i, type(fs).__name__) for i, fs in enumerate(flow_systems) if not isinstance(fs, FlowSystem)] + if non_fs: + raise TypeError(f'flow_systems must contain only FlowSystem instances; got {non_fs} (index, type)') + + self._systems: list[FlowSystem] = flow_systems self._names = names or [fs.name or f'System {i}' for i, fs in enumerate(flow_systems)] if len(self._names) != len(self._systems): diff --git a/tests/test_comparison.py b/tests/test_comparison.py index f470e8760..94328da97 100644 --- a/tests/test_comparison.py +++ b/tests/test_comparison.py @@ -173,6 +173,16 @@ def test_comparison_rejects_unoptimized_system(self, base_flow_system, optimized with pytest.raises(RuntimeError, match='no solution'): _ = comp.solution + def test_comparison_rejects_non_list(self, optimized_base, optimized_with_chp): + """Comparison rejects non-list flow_systems input.""" + with pytest.raises(TypeError, match='must be a list'): + fx.Comparison((optimized_base, optimized_with_chp)) + + def test_comparison_rejects_non_flowsystem_items(self, optimized_base): + """Comparison rejects list items that are not FlowSystem instances.""" + with pytest.raises(TypeError, match='FlowSystem instances'): + fx.Comparison([optimized_base, 'not a flow system']) + # ============================================================================ # CONTAINER PROTOCOL TESTS From 644c44dec7fbdfee219e0136fa469ac5a3e7e220 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 20 Apr 2026 12:48:47 +0200 Subject: [PATCH 4/4] fix: validate element types before length in Comparison Check that each element is a FlowSystem before the minimum-length check, so `Comparison([not_a_flow_system])` surfaces the real problem (wrong type) instead of "requires at least 2 FlowSystems". Co-Authored-By: Claude Opus 4.7 (1M context) --- flixopt/comparison.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flixopt/comparison.py b/flixopt/comparison.py index d35d5a49b..a8c2076c8 100644 --- a/flixopt/comparison.py +++ b/flixopt/comparison.py @@ -163,13 +163,13 @@ def __init__(self, flow_systems: list[FlowSystem], names: list[str] | None = Non if not isinstance(flow_systems, list): raise TypeError(f'flow_systems must be a list, got {type(flow_systems).__name__}') - if len(flow_systems) < 2: - raise ValueError('Comparison requires at least 2 FlowSystems') - non_fs = [(i, type(fs).__name__) for i, fs in enumerate(flow_systems) if not isinstance(fs, FlowSystem)] if non_fs: raise TypeError(f'flow_systems must contain only FlowSystem instances; got {non_fs} (index, type)') + if len(flow_systems) < 2: + raise ValueError('Comparison requires at least 2 FlowSystems') + self._systems: list[FlowSystem] = flow_systems self._names = names or [fs.name or f'System {i}' for i, fs in enumerate(flow_systems)]