From 3b24b31409d942fe9b74791bed0b0bcb692431b2 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Sun, 24 May 2026 17:16:48 +0200 Subject: [PATCH 01/13] feat: Variable.update() / Constraint.update() as canonical mutation API Introduces typed ``.update()`` methods on Variable and Constraint as the single, validated, multi-attribute mutation entry point. - ``Variable.update(lower=, upper=)``: validates non-constant inputs are rejected, new dims are rejected, and the resulting ``lower <= upper`` invariant holds across all coords. Returns ``self`` for chaining. - ``Constraint.update(rhs=, sign=)``: constant RHS only. The legacy ``c.rhs = variable`` rearrange-to-lhs path stays on the setter (different semantic, deserves its own explicit method). The existing ``.lower`` / ``.upper`` / ``.sign`` setters become thin shims that forward to ``.update()``, so single-attribute writes (``z.lower = 2``) stay ergonomic and the canonical validation runs in one place. The ``.rhs`` setter forwards constants through ``.update()`` and keeps the expression-rhs rearrange behaviour. This is the on-top experiment for the design discussion on #718. ``.lhs`` / ``.coeffs`` / ``.vars`` setters are untouched for now. Co-Authored-By: Claude Opus 4.7 (1M context) --- linopy/constraints.py | 85 ++++++++++++++++++++++++++++++++---- linopy/variables.py | 95 ++++++++++++++++++++++++++++++++--------- test/test_constraint.py | 30 +++++++++++++ test/test_variable.py | 39 +++++++++++++++++ 4 files changed, 221 insertions(+), 28 deletions(-) diff --git a/linopy/constraints.py b/linopy/constraints.py index 6f11b137..97039d0e 100644 --- a/linopy/constraints.py +++ b/linopy/constraints.py @@ -55,7 +55,6 @@ maybe_group_terms_polars, maybe_replace_signs, replace_by_map, - require_constant, save_join, to_dataframe, to_polars, @@ -1143,10 +1142,9 @@ def sign(self) -> DataArray: return self.data.sign @sign.setter - @require_constant def sign(self, value: SignLike) -> None: - value = maybe_replace_signs(DataArray(value)).broadcast_like(self.sign) - self._data = assign_multiindex_safe(self.data, sign=value) + """Shim — forwards to :meth:`Constraint.update`.""" + self.update(sign=value) @property def rhs(self) -> DataArray: @@ -1154,15 +1152,86 @@ def rhs(self) -> DataArray: @rhs.setter def rhs(self, value: ExpressionLike | VariableLike | ConstantLike) -> None: - value = expressions.as_expression( + """ + Set RHS. Constants go through :meth:`Constraint.update`; non-constant + rhs (Variable/Expression) is rearranged to LHS in place. + """ + # Constant path: route through the canonical update API. + if not isinstance( + value, + variables.Variable + | variables.ScalarVariable + | expressions.LinearExpression + | expressions.QuadraticExpression, + ): + self.update(rhs=value) + return + # Non-constant rhs: existing rearrange-to-lhs behaviour (kept for + # backward compatibility; an explicit method would be cleaner). + expr = expressions.as_expression( value, self.model, coords=self.coords, dims=self.coord_dims ) - residual = value.reset_const() + residual = expr.reset_const() if residual.nterm == 0: - self._data = assign_multiindex_safe(self.data, rhs=value.const) + self._data = assign_multiindex_safe(self.data, rhs=expr.const) return self.lhs = self.lhs - residual - self._data = assign_multiindex_safe(self.data, rhs=value.const) + self._data = assign_multiindex_safe(self.data, rhs=expr.const) + + def update( + self, + *, + rhs: ConstantLike | None = None, + sign: SignLike | None = None, + ) -> Constraint: + """ + Update the constraint's RHS and/or sign in place. + + Canonical mutation API. Single-attribute setters (`c.rhs = …`, + `c.sign = …`) forward to this method. + + Parameters + ---------- + rhs : ConstantLike, optional + New constant RHS. Variable / Expression RHS is not supported + here; use the (legacy) ``c.rhs = expression`` setter, which + rearranges the residual into ``c.lhs``. + sign : SignLike, optional + New sign. One of ``"<=" / "==" / ">="`` (or their ``< > =`` + aliases). + + Returns + ------- + Constraint + ``self`` for chaining. + """ + if rhs is None and sign is None: + return self + + if rhs is not None and isinstance( + rhs, + variables.Variable + | variables.ScalarVariable + | expressions.LinearExpression + | expressions.QuadraticExpression, + ): + raise TypeError( + "Constraint.update(rhs=...) only accepts constants; " + f"got {type(rhs).__name__}. Use the legacy `c.rhs = expr` " + "setter or rebuild via add_constraints(...) for " + "expression / Variable rhs." + ) + + updates: dict[str, DataArray] = {} + if rhs is not None: + new_rhs = DataArray(rhs).broadcast_like(self.rhs) + updates["rhs"] = new_rhs + if sign is not None: + new_sign = maybe_replace_signs(DataArray(sign)).broadcast_like(self.sign) + updates["sign"] = new_sign + + self._data = assign_multiindex_safe(self.data, **updates) + return self @property def lhs(self) -> expressions.LinearExpression: diff --git a/linopy/variables.py b/linopy/variables.py index cbf2fb87..e416b417 100644 --- a/linopy/variables.py +++ b/linopy/variables.py @@ -48,7 +48,6 @@ get_label_position, has_optimized_model, iterate_slices, - require_constant, save_join, set_int_index, to_dataframe, @@ -891,18 +890,9 @@ def upper(self) -> DataArray: return self.data.upper @upper.setter - @require_constant def upper(self, value: ConstantLike) -> None: - """ - Set the upper bounds of the variables. - - The function raises an error in case no model is set as a - reference. - """ - value = DataArray(value).broadcast_like(self.upper) - if not set(value.dims).issubset(self.model.variables[self.name].dims): - raise ValueError("Cannot assign new dimensions to existing variable.") - self._data = assign_multiindex_safe(self.data, upper=value) + """Shim — forwards to :meth:`Variable.update`.""" + self.update(upper=value) @property def lower(self) -> DataArray: @@ -915,18 +905,83 @@ def lower(self) -> DataArray: return self.data.lower @lower.setter - @require_constant def lower(self, value: ConstantLike) -> None: + """Shim — forwards to :meth:`Variable.update`.""" + self.update(lower=value) + + def update( + self, + *, + lower: ConstantLike | None = None, + upper: ConstantLike | None = None, + ) -> Variable: """ - Set the lower bounds of the variables. + Update variable bounds in place. - The function raises an error in case no model is set as a - reference. + Canonical mutation API. Validation and coord alignment live here. + Single-attribute setters (`var.lower = …`) forward to this method. + + Parameters + ---------- + lower : ConstantLike, optional + New lower bound. Aligned via xarray broadcast against the + variable's existing shape; new dims are rejected. + upper : ConstantLike, optional + New upper bound. Same. + + Returns + ------- + Variable + ``self`` for chaining. + + Raises + ------ + TypeError + If either bound is a Variable / Expression instead of a constant. + ValueError + If the new bound introduces dimensions not in the variable's + coords, or if the resulting ``lower > upper`` anywhere. """ - value = DataArray(value).broadcast_like(self.lower) - if not set(value.dims).issubset(self.model.variables[self.name].dims): - raise ValueError("Cannot assign new dimensions to existing variable.") - self._data = assign_multiindex_safe(self.data, lower=value) + if lower is None and upper is None: + return self + + from linopy import expressions + + non_constant = ( + Variable, + ScalarVariable, + expressions.LinearExpression, + expressions.QuadraticExpression, + ) + for name, val in (("lower", lower), ("upper", upper)): + if val is not None and isinstance(val, non_constant): + raise TypeError( + f"Variable.update({name}=...) must be a constant; " + f"got {type(val).__name__}." + ) + + updates: dict[str, DataArray] = {} + own_dims = self.model.variables[self.name].dims + if lower is not None: + new_lower = DataArray(lower).broadcast_like(self.lower) + if not set(new_lower.dims).issubset(own_dims): + raise ValueError("Cannot assign new dimensions to existing variable.") + updates["lower"] = new_lower + if upper is not None: + new_upper = DataArray(upper).broadcast_like(self.upper) + if not set(new_upper.dims).issubset(own_dims): + raise ValueError("Cannot assign new dimensions to existing variable.") + updates["upper"] = new_upper + + final_lower = updates.get("lower", self.lower) + final_upper = updates.get("upper", self.upper) + if bool((final_lower > final_upper).any()): + raise ValueError( + "Variable.update would leave lower > upper at one or more coordinates." + ) + + self._data = assign_multiindex_safe(self.data, **updates) + return self @property @has_optimized_model diff --git a/test/test_constraint.py b/test/test_constraint.py index 690da8f6..d69ebb02 100644 --- a/test/test_constraint.py +++ b/test/test_constraint.py @@ -426,6 +426,36 @@ def test_constraint_rhs_setter(mc: linopy.constraints.Constraint) -> None: assert mc.sizes == sizes +def test_constraint_update_rhs_and_sign(mc: linopy.constraints.Constraint) -> None: + mc.update(rhs=5, sign=EQUAL) + assert (mc.rhs == 5).all() + assert (mc.sign == EQUAL).all() + + +def test_constraint_update_no_kwargs_is_noop( + mc: linopy.constraints.Constraint, +) -> None: + old_rhs = mc.rhs.copy() + old_sign = mc.sign.copy() + mc.update() + assert (mc.rhs == old_rhs).all() + assert (mc.sign == old_sign).all() + + +def test_constraint_update_rejects_variable_rhs( + mc: linopy.constraints.Constraint, x: linopy.Variable +) -> None: + with pytest.raises(TypeError, match="only accepts constants"): + mc.update(rhs=x) + + +def test_constraint_update_returns_self( + mc: linopy.constraints.Constraint, +) -> None: + out = mc.update(rhs=7) + assert out is mc + + def test_constraint_rhs_setter_with_variable( mc: linopy.constraints.Constraint, x: linopy.Variable ) -> None: diff --git a/test/test_variable.py b/test/test_variable.py index b14b746e..8064703b 100644 --- a/test/test_variable.py +++ b/test/test_variable.py @@ -186,6 +186,45 @@ def test_variable_lower_setter_with_array_invalid_dim(x: linopy.Variable) -> Non x.lower = lower +def test_variable_update_bounds(z: linopy.Variable) -> None: + z.update(lower=2, upper=20) + assert z.lower.item() == 2 + assert z.upper.item() == 20 + + +def test_variable_update_lower_only(z: linopy.Variable) -> None: + z.update(lower=3) + assert z.lower.item() == 3 + assert z.upper.item() == 10 # unchanged from fixture default + + +def test_variable_update_no_kwargs_is_noop(z: linopy.Variable) -> None: + old_lower, old_upper = z.lower.item(), z.upper.item() + z.update() + assert z.lower.item() == old_lower + assert z.upper.item() == old_upper + + +def test_variable_update_rejects_inverted_bounds(z: linopy.Variable) -> None: + with pytest.raises(ValueError, match="lower > upper"): + z.update(lower=20, upper=5) + + +def test_variable_update_rejects_non_constant(z: linopy.Variable) -> None: + with pytest.raises(TypeError, match="must be a constant"): + z.update(upper=z) + + +def test_variable_update_returns_self(z: linopy.Variable) -> None: + out = z.update(lower=1) + assert out is z + + +def test_variable_update_array_invalid_dim(x: linopy.Variable) -> None: + with pytest.raises(ValueError): + x.update(lower=pd.Series(range(15, 25))) + + def test_variable_sum(x: linopy.Variable) -> None: res = x.sum() assert res.nterm == 10 From 0d48e84bcf47ff2825df0da6945f7af3c4638ee2 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Sun, 24 May 2026 18:43:18 +0200 Subject: [PATCH 02/13] feat(update): Constraint.update accepts Variable/Expression rhs Mirrors the existing ``c.rhs = expr`` setter and ``add_constraints`` which both accept mixed-side input and rearrange the residual onto lhs. ``c.update(rhs=x + 5)`` now subtracts ``x`` from lhs and stores ``5`` on rhs. ``.rhs`` setter collapses to a one-line shim. Variable bound rejection of Variable/Expression is kept (bounds are numeric, not symbolic); docstring clarified to spell out that pandas / xarray / numpy arrays are first-class (time-varying bounds). Co-Authored-By: Claude Opus 4.7 (1M context) --- linopy/constraints.py | 69 ++++++++++++++--------------------------- linopy/variables.py | 10 ++++-- test/test_constraint.py | 11 +++++-- 3 files changed, 38 insertions(+), 52 deletions(-) diff --git a/linopy/constraints.py b/linopy/constraints.py index 97039d0e..029ea894 100644 --- a/linopy/constraints.py +++ b/linopy/constraints.py @@ -1152,36 +1152,13 @@ def rhs(self) -> DataArray: @rhs.setter def rhs(self, value: ExpressionLike | VariableLike | ConstantLike) -> None: - """ - Set RHS. Constants go through :meth:`Constraint.update`; non-constant - rhs (Variable/Expression) is rearranged to LHS in place. - """ - # Constant path: route through the canonical update API. - if not isinstance( - value, - variables.Variable - | variables.ScalarVariable - | expressions.LinearExpression - | expressions.QuadraticExpression, - ): - self.update(rhs=value) - return - # Non-constant rhs: existing rearrange-to-lhs behaviour (kept for - # backward compatibility; an explicit method would be cleaner). - expr = expressions.as_expression( - value, self.model, coords=self.coords, dims=self.coord_dims - ) - residual = expr.reset_const() - if residual.nterm == 0: - self._data = assign_multiindex_safe(self.data, rhs=expr.const) - return - self.lhs = self.lhs - residual - self._data = assign_multiindex_safe(self.data, rhs=expr.const) + """Shim — forwards to :meth:`Constraint.update`.""" + self.update(rhs=value) def update( self, *, - rhs: ConstantLike | None = None, + rhs: ExpressionLike | VariableLike | ConstantLike | None = None, sign: SignLike | None = None, ) -> Constraint: """ @@ -1192,10 +1169,11 @@ def update( Parameters ---------- - rhs : ConstantLike, optional - New constant RHS. Variable / Expression RHS is not supported - here; use the (legacy) ``c.rhs = expression`` setter, which - rearranges the residual into ``c.lhs``. + rhs : ExpressionLike / VariableLike / ConstantLike, optional + New right-hand side. Variable / Expression rhs is rearranged + onto the lhs (matching ``add_constraints`` and the legacy + ``c.rhs = expr`` setter): the residual is subtracted from + ``c.lhs`` and only the constant part lands on ``c.rhs``. sign : SignLike, optional New sign. One of ``"<=" / "==" / ">="`` (or their ``< > =`` aliases). @@ -1208,27 +1186,26 @@ def update( if rhs is None and sign is None: return self - if rhs is not None and isinstance( - rhs, - variables.Variable - | variables.ScalarVariable - | expressions.LinearExpression - | expressions.QuadraticExpression, - ): - raise TypeError( - "Constraint.update(rhs=...) only accepts constants; " - f"got {type(rhs).__name__}. Use the legacy `c.rhs = expr` " - "setter or rebuild via add_constraints(...) for " - "expression / Variable rhs." + if rhs is not None: + expr = expressions.as_expression( + rhs, self.model, coords=self.coords, dims=self.coord_dims ) + residual = expr.reset_const() + if residual.nterm != 0: + # Move the non-constant part of `rhs` onto lhs, then store + # the constant part on rhs. + self.lhs = self.lhs - residual + new_rhs = expr.const + else: + new_rhs = None updates: dict[str, DataArray] = {} - if rhs is not None: - new_rhs = DataArray(rhs).broadcast_like(self.rhs) + if new_rhs is not None: updates["rhs"] = new_rhs if sign is not None: - new_sign = maybe_replace_signs(DataArray(sign)).broadcast_like(self.sign) - updates["sign"] = new_sign + updates["sign"] = maybe_replace_signs(DataArray(sign)).broadcast_like( + self.sign + ) self._data = assign_multiindex_safe(self.data, **updates) return self diff --git a/linopy/variables.py b/linopy/variables.py index e416b417..04a1714f 100644 --- a/linopy/variables.py +++ b/linopy/variables.py @@ -924,8 +924,11 @@ def update( Parameters ---------- lower : ConstantLike, optional - New lower bound. Aligned via xarray broadcast against the - variable's existing shape; new dims are rejected. + New lower bound. Accepts any constant — scalars, numpy + arrays, pandas Series / DataFrame, xarray DataArray (e.g. + time-varying bounds). Aligned via xarray broadcast against + the variable's existing shape; new dims are rejected. + Decision variables / linear expressions are not accepted. upper : ConstantLike, optional New upper bound. Same. @@ -937,7 +940,8 @@ def update( Raises ------ TypeError - If either bound is a Variable / Expression instead of a constant. + If either bound is a Variable / Expression (bounds must be + numeric, not symbolic). ValueError If the new bound introduces dimensions not in the variable's coords, or if the resulting ``lower > upper`` anywhere. diff --git a/test/test_constraint.py b/test/test_constraint.py index d69ebb02..82f6fab8 100644 --- a/test/test_constraint.py +++ b/test/test_constraint.py @@ -442,11 +442,16 @@ def test_constraint_update_no_kwargs_is_noop( assert (mc.sign == old_sign).all() -def test_constraint_update_rejects_variable_rhs( +def test_constraint_update_rearranges_variable_rhs( mc: linopy.constraints.Constraint, x: linopy.Variable ) -> None: - with pytest.raises(TypeError, match="only accepts constants"): - mc.update(rhs=x) + """ + Variable / Expression rhs is moved onto lhs; only the constant + part lands on rhs (mirrors add_constraints and the .rhs setter). + """ + mc.update(rhs=x + 3) + assert (mc.rhs == 3).all() + assert mc.lhs.nterm == 2 # original term + the rearranged -x def test_constraint_update_returns_self( From 7675b66ead45f10ae9e0daf23da342742fe20ce8 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 25 May 2026 01:23:12 +0200 Subject: [PATCH 03/13] feat(update): extend Constraint.update to lhs/coeffs/vars; shim all setters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds lhs / coeffs / vars to the canonical mutation API. All .lhs / .coeffs / .vars setters now forward to .update() — every Constraint mutation goes through one method with one validation path, one place that flips _coef_dirty. Composition rules: - lhs= replaces the whole expression first; subsequent rhs= rearrangement (Variable/Expression in rhs) sees the new lhs. - lhs= and coeffs= / vars= are mutually exclusive (whole replacement vs partial array update). - sign= is applied last so it composes cleanly. Internal Constraint.sanitize_zeros migrated to update(vars=, coeffs=) — no more internal setter calls in linopy/. 389 tests pass across mutation + persistent-solver suite. Co-Authored-By: Claude Opus 4.7 (1M context) --- linopy/constraints.py | 142 ++++++++++++++++++++++++++++-------------- 1 file changed, 94 insertions(+), 48 deletions(-) diff --git a/linopy/constraints.py b/linopy/constraints.py index 029ea894..25a7be86 100644 --- a/linopy/constraints.py +++ b/linopy/constraints.py @@ -1119,9 +1119,8 @@ def coeffs(self) -> DataArray: @coeffs.setter def coeffs(self, value: ConstantLike) -> None: - value = DataArray(value).broadcast_like(self.vars, exclude=[self.term_dim]) - self._data = assign_multiindex_safe(self.data, coeffs=value) - self._coef_dirty = True + """Shim — forwards to :meth:`Constraint.update`.""" + self.update(coeffs=value) @property def vars(self) -> DataArray: @@ -1129,13 +1128,8 @@ def vars(self) -> DataArray: @vars.setter def vars(self, value: variables.Variable | DataArray) -> None: - if isinstance(value, variables.Variable): - value = value.labels - if not isinstance(value, DataArray): - raise TypeError("Expected value to be of type DataArray or Variable") - value = value.broadcast_like(self.coeffs, exclude=[self.term_dim]) - self._data = assign_multiindex_safe(self.data, vars=value) - self._coef_dirty = True + """Shim — forwards to :meth:`Constraint.update`.""" + self.update(vars=value) @property def sign(self) -> DataArray: @@ -1155,76 +1149,127 @@ def rhs(self, value: ExpressionLike | VariableLike | ConstantLike) -> None: """Shim — forwards to :meth:`Constraint.update`.""" self.update(rhs=value) + @property + def lhs(self) -> expressions.LinearExpression: + data = self.data[["coeffs", "vars"]].rename({self.term_dim: TERM_DIM}) + return expressions.LinearExpression(data, self.model) + + @lhs.setter + def lhs(self, value: ExpressionLike | VariableLike | ConstantLike) -> None: + """Shim — forwards to :meth:`Constraint.update`.""" + self.update(lhs=value) + + def _assign_lhs_expr( + self, expr: expressions.LinearExpression, rhs: DataArray | None = None + ) -> None: + """ + Internal: replace coeffs/vars from ``expr``, adjusting rhs for + the expression's constant part. Sets ``_coef_dirty``. + """ + base_rhs = self.rhs if rhs is None else rhs + self._data = self.data.drop_vars(["coeffs", "vars"]).assign( + coeffs=expr.coeffs, + vars=expr.vars, + rhs=base_rhs - expr.const, + ) + self._coef_dirty = True + def update( self, *, + lhs: ExpressionLike | VariableLike | ConstantLike | None = None, rhs: ExpressionLike | VariableLike | ConstantLike | None = None, sign: SignLike | None = None, + coeffs: ConstantLike | None = None, + vars: variables.Variable | DataArray | None = None, ) -> Constraint: """ - Update the constraint's RHS and/or sign in place. + Update the constraint in place. - Canonical mutation API. Single-attribute setters (`c.rhs = …`, - `c.sign = …`) forward to this method. + The only mutation API; setters forward here. All keyword + arguments are optional; pass only what you want to change. Parameters ---------- + lhs : ExpressionLike / VariableLike / ConstantLike, optional + Replace the LHS expression. Any constant part is moved to + ``rhs`` so ``c.lhs`` stays pure-variable. Cannot be combined + with ``coeffs`` / ``vars``. Sets the internal + ``_coef_dirty`` flag. rhs : ExpressionLike / VariableLike / ConstantLike, optional New right-hand side. Variable / Expression rhs is rearranged - onto the lhs (matching ``add_constraints`` and the legacy - ``c.rhs = expr`` setter): the residual is subtracted from - ``c.lhs`` and only the constant part lands on ``c.rhs``. + onto the lhs (matching ``add_constraints``): the residual is + subtracted from ``c.lhs`` and only the constant part lands + on ``c.rhs``. sign : SignLike, optional New sign. One of ``"<=" / "==" / ">="`` (or their ``< > =`` aliases). + coeffs : ConstantLike, optional + Replace coefficient values (same sparsity / term structure). + Lower-level than ``lhs=``; sets ``_coef_dirty``. + vars : Variable / DataArray, optional + Replace variable label array (same sparsity / term + structure). Lower-level than ``lhs=``; sets ``_coef_dirty``. Returns ------- Constraint ``self`` for chaining. """ - if rhs is None and sign is None: + if all(v is None for v in (lhs, rhs, sign, coeffs, vars)): return self + if lhs is not None and (coeffs is not None or vars is not None): + raise TypeError( + "Constraint.update: pass either `lhs=` (replace the whole " + "expression) or `coeffs=` / `vars=` (partial array " + "replacement), not both." + ) + + # 1. lhs replacement first so subsequent rhs= rearrangement sees the new lhs. + if lhs is not None: + self._assign_lhs_expr( + expressions.as_expression( + lhs, self.model, coords=self.coords, dims=self.coord_dims + ) + ) + + # 2. rhs (rearranges non-constant part onto lhs). if rhs is not None: expr = expressions.as_expression( rhs, self.model, coords=self.coords, dims=self.coord_dims ) residual = expr.reset_const() if residual.nterm != 0: - # Move the non-constant part of `rhs` onto lhs, then store - # the constant part on rhs. - self.lhs = self.lhs - residual - new_rhs = expr.const - else: - new_rhs = None + self._assign_lhs_expr(self.lhs - residual, rhs=expr.const) + else: + self._data = assign_multiindex_safe(self.data, rhs=expr.const) - updates: dict[str, DataArray] = {} - if new_rhs is not None: - updates["rhs"] = new_rhs - if sign is not None: - updates["sign"] = maybe_replace_signs(DataArray(sign)).broadcast_like( - self.sign + # 3. coeffs / vars partial updates (only valid without lhs=). + if coeffs is not None: + new_coeffs = DataArray(coeffs).broadcast_like( + self.vars, exclude=[self.term_dim] ) + self._data = assign_multiindex_safe(self.data, coeffs=new_coeffs) + self._coef_dirty = True + if vars is not None: + v = vars.labels if isinstance(vars, variables.Variable) else vars + if not isinstance(v, DataArray): + raise TypeError( + "Constraint.update(vars=...) expects a DataArray or " + f"Variable; got {type(vars).__name__}." + ) + new_vars = v.broadcast_like(self.coeffs, exclude=[self.term_dim]) + self._data = assign_multiindex_safe(self.data, vars=new_vars) + self._coef_dirty = True + + # 4. sign last so it composes cleanly with the rest. + if sign is not None: + new_sign = maybe_replace_signs(DataArray(sign)).broadcast_like(self.sign) + self._data = assign_multiindex_safe(self.data, sign=new_sign) - self._data = assign_multiindex_safe(self.data, **updates) return self - @property - def lhs(self) -> expressions.LinearExpression: - data = self.data[["coeffs", "vars"]].rename({self.term_dim: TERM_DIM}) - return expressions.LinearExpression(data, self.model) - - @lhs.setter - def lhs(self, value: ExpressionLike | VariableLike | ConstantLike) -> None: - value = expressions.as_expression( - value, self.model, coords=self.coords, dims=self.coord_dims - ) - self._data = self.data.drop_vars(["coeffs", "vars"]).assign( - coeffs=value.coeffs, vars=value.vars, rhs=self.rhs - value.const - ) - self._coef_dirty = True - @property @has_optimized_model def dual(self) -> DataArray: @@ -1327,9 +1372,10 @@ def to_matrix_with_rhs( def sanitize_zeros(self) -> Constraint: """Remove terms with zero or near-zero coefficients.""" not_zero = abs(self.coeffs) > 1e-10 - self.vars = self.vars.where(not_zero, -1) - self.coeffs = self.coeffs.where(not_zero) - return self + return self.update( + vars=self.vars.where(not_zero, -1), + coeffs=self.coeffs.where(not_zero), + ) def sanitize_missings(self) -> Constraint: """Mask out rows where all variables are missing (-1).""" From 3b716b52c5275ede4a1f5cb8f39f2ae86902f5c0 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 25 May 2026 01:30:58 +0200 Subject: [PATCH 04/13] feat(update): rename Constraint.update kwarg vars= -> variables= Avoids shadowing Python's vars() builtin. The .vars attribute on Constraint stays (it parallels the .data.vars internal name); only the kwarg gets the unambiguous spelling. Co-Authored-By: Claude Opus 4.7 (1M context) --- linopy/constraints.py | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/linopy/constraints.py b/linopy/constraints.py index 25a7be86..2a33a48a 100644 --- a/linopy/constraints.py +++ b/linopy/constraints.py @@ -1129,7 +1129,7 @@ def vars(self) -> DataArray: @vars.setter def vars(self, value: variables.Variable | DataArray) -> None: """Shim — forwards to :meth:`Constraint.update`.""" - self.update(vars=value) + self.update(variables=value) @property def sign(self) -> DataArray: @@ -1181,7 +1181,7 @@ def update( rhs: ExpressionLike | VariableLike | ConstantLike | None = None, sign: SignLike | None = None, coeffs: ConstantLike | None = None, - vars: variables.Variable | DataArray | None = None, + variables: variables.Variable | DataArray | None = None, ) -> Constraint: """ Update the constraint in place. @@ -1194,7 +1194,7 @@ def update( lhs : ExpressionLike / VariableLike / ConstantLike, optional Replace the LHS expression. Any constant part is moved to ``rhs`` so ``c.lhs`` stays pure-variable. Cannot be combined - with ``coeffs`` / ``vars``. Sets the internal + with ``coeffs`` / ``variables``. Sets the internal ``_coef_dirty`` flag. rhs : ExpressionLike / VariableLike / ConstantLike, optional New right-hand side. Variable / Expression rhs is rearranged @@ -1207,22 +1207,24 @@ def update( coeffs : ConstantLike, optional Replace coefficient values (same sparsity / term structure). Lower-level than ``lhs=``; sets ``_coef_dirty``. - vars : Variable / DataArray, optional + variables : Variable / DataArray, optional Replace variable label array (same sparsity / term structure). Lower-level than ``lhs=``; sets ``_coef_dirty``. + Mirrors the ``c.vars`` attribute; spelled out here to avoid + shadowing Python's ``vars()`` builtin in the kwarg name. Returns ------- Constraint ``self`` for chaining. """ - if all(v is None for v in (lhs, rhs, sign, coeffs, vars)): + if all(v is None for v in (lhs, rhs, sign, coeffs, variables)): return self - if lhs is not None and (coeffs is not None or vars is not None): + if lhs is not None and (coeffs is not None or variables is not None): raise TypeError( "Constraint.update: pass either `lhs=` (replace the whole " - "expression) or `coeffs=` / `vars=` (partial array " + "expression) or `coeffs=` / `variables=` (partial array " "replacement), not both." ) @@ -1245,19 +1247,21 @@ def update( else: self._data = assign_multiindex_safe(self.data, rhs=expr.const) - # 3. coeffs / vars partial updates (only valid without lhs=). + # 3. coeffs / variables partial updates (only valid without lhs=). if coeffs is not None: new_coeffs = DataArray(coeffs).broadcast_like( self.vars, exclude=[self.term_dim] ) self._data = assign_multiindex_safe(self.data, coeffs=new_coeffs) self._coef_dirty = True - if vars is not None: - v = vars.labels if isinstance(vars, variables.Variable) else vars + if variables is not None: + from linopy.variables import Variable as _Variable + + v = variables.labels if isinstance(variables, _Variable) else variables if not isinstance(v, DataArray): raise TypeError( - "Constraint.update(vars=...) expects a DataArray or " - f"Variable; got {type(vars).__name__}." + "Constraint.update(variables=...) expects a DataArray or " + f"Variable; got {type(variables).__name__}." ) new_vars = v.broadcast_like(self.coeffs, exclude=[self.term_dim]) self._data = assign_multiindex_safe(self.data, vars=new_vars) @@ -1373,7 +1377,7 @@ def sanitize_zeros(self) -> Constraint: """Remove terms with zero or near-zero coefficients.""" not_zero = abs(self.coeffs) > 1e-10 return self.update( - vars=self.vars.where(not_zero, -1), + variables=self.vars.where(not_zero, -1), coeffs=self.coeffs.where(not_zero), ) From 4a4276f899bc14f450b8fd5fad78af3bfbb05a2a Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 25 May 2026 11:19:31 +0200 Subject: [PATCH 05/13] feat(update): accept positional ConstraintLike in Constraint.update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors add_constraints' dispatch: c.update(x + 5 <= 3) is now shorthand for c.update(lhs=x, sign='<=', rhs=-2), extracted from the AnonymousConstraint / ConstraintBase the comparison produces. Mutually exclusive with the per-attribute kwargs; clear error when mixed. Also reverts the internal sanitize_zeros migration. The setters are pure shims forwarding to update(), so the migration didn't change behaviour or cost — just spelling. The original setter syntax reads more naturally there. Co-Authored-By: Claude Opus 4.7 (1M context) --- linopy/constraints.py | 39 +++++++++++++++++++++++++++++++++------ test/test_constraint.py | 26 ++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/linopy/constraints.py b/linopy/constraints.py index 2a33a48a..dd712add 100644 --- a/linopy/constraints.py +++ b/linopy/constraints.py @@ -71,6 +71,7 @@ ) from linopy.types import ( ConstantLike, + ConstraintLike, CoordsLike, ExpressionLike, SignLike, @@ -1176,6 +1177,7 @@ def _assign_lhs_expr( def update( self, + constraint: ConstraintLike | None = None, *, lhs: ExpressionLike | VariableLike | ConstantLike | None = None, rhs: ExpressionLike | VariableLike | ConstantLike | None = None, @@ -1186,11 +1188,19 @@ def update( """ Update the constraint in place. - The only mutation API; setters forward here. All keyword - arguments are optional; pass only what you want to change. + The only mutation API; setters forward here. Two call shapes: + + * ``c.update(x + 5 <= 3)`` — pass a complete constraint + expression (mirroring ``add_constraints``). Replaces lhs, + sign, and rhs at once. + * ``c.update(lhs=, rhs=, sign=, coeffs=, variables=)`` — pass + only what you want to change. Parameters ---------- + constraint : ConstraintLike, optional + A complete constraint expression (e.g. ``x + 5 <= 3``). + Mutually exclusive with the keyword arguments below. lhs : ExpressionLike / VariableLike / ConstantLike, optional Replace the LHS expression. Any constant part is moved to ``rhs`` so ``c.lhs`` stays pure-variable. Cannot be combined @@ -1218,6 +1228,24 @@ def update( Constraint ``self`` for chaining. """ + if constraint is not None: + if any(x is not None for x in (lhs, rhs, sign, coeffs, variables)): + raise TypeError( + "Constraint.update: positional `constraint` argument " + "cannot be combined with keyword arguments." + ) + if isinstance(constraint, AnonymousScalarConstraint): + con = constraint.to_constraint() + elif isinstance(constraint, ConstraintBase): + con = constraint + else: + raise TypeError( + "Constraint.update: positional argument must be a " + "ConstraintLike (e.g. `x + 5 <= 3`); got " + f"{type(constraint).__name__}." + ) + lhs, sign, rhs = con.lhs, con.sign, con.rhs + if all(v is None for v in (lhs, rhs, sign, coeffs, variables)): return self @@ -1376,10 +1404,9 @@ def to_matrix_with_rhs( def sanitize_zeros(self) -> Constraint: """Remove terms with zero or near-zero coefficients.""" not_zero = abs(self.coeffs) > 1e-10 - return self.update( - variables=self.vars.where(not_zero, -1), - coeffs=self.coeffs.where(not_zero), - ) + self.vars = self.vars.where(not_zero, -1) + self.coeffs = self.coeffs.where(not_zero) + return self def sanitize_missings(self) -> Constraint: """Mask out rows where all variables are missing (-1).""" diff --git a/test/test_constraint.py b/test/test_constraint.py index 82f6fab8..5fb6d29a 100644 --- a/test/test_constraint.py +++ b/test/test_constraint.py @@ -461,6 +461,32 @@ def test_constraint_update_returns_self( assert out is mc +def test_constraint_update_positional_constraint_expression( + mc: linopy.constraints.Constraint, x: linopy.Variable, y: linopy.Variable +) -> None: + """``c.update(x + 5 <= 3)`` replaces lhs / sign / rhs in one call.""" + mc.update(x + y <= 7) + assert (mc.rhs == 7).all() + assert (mc.sign == LESS_EQUAL).all() + assert mc.lhs.nterm == 2 + + +def test_constraint_update_positional_rejects_mixing_kwargs( + mc: linopy.constraints.Constraint, x: linopy.Variable +) -> None: + """Positional constraint can't be combined with keyword updates.""" + with pytest.raises(TypeError, match="cannot be combined with keyword"): + mc.update(x <= 3, sign=EQUAL) + + +def test_constraint_update_positional_rejects_non_constraint( + mc: linopy.constraints.Constraint, +) -> None: + """Random objects are rejected with a clear error.""" + with pytest.raises(TypeError, match="must be a ConstraintLike"): + mc.update("not a constraint") # type: ignore + + def test_constraint_rhs_setter_with_variable( mc: linopy.constraints.Constraint, x: linopy.Variable ) -> None: From 33e2d07f84f58b8bb0201e5d627c092c28cb838e Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 25 May 2026 11:29:05 +0200 Subject: [PATCH 06/13] docs(update): note kwarg form is the targeted, cheap path The positional ConstraintLike form (c.update(x + 5 <= 3)) always rewrites lhs / sign / rhs and flips _coef_dirty. For hot loops that only touch one part, kwarg form (c.update(rhs=...)) skips the unchanged attributes and is materially cheaper. Co-Authored-By: Claude Opus 4.7 (1M context) --- linopy/constraints.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/linopy/constraints.py b/linopy/constraints.py index dd712add..066614ac 100644 --- a/linopy/constraints.py +++ b/linopy/constraints.py @@ -1196,6 +1196,22 @@ def update( * ``c.update(lhs=, rhs=, sign=, coeffs=, variables=)`` — pass only what you want to change. + Use the keyword form for targeted changes — it skips the + unchanged attributes entirely. The positional form always + rewrites lhs / sign / rhs (and flips ``_coef_dirty``), so it + is the wrong shape for hot loops that only touch one part: + + .. code-block:: python + + # Hot loop, rhs is the only thing changing per iteration: + for k in scenarios: + c.update(rhs=rhs_k) # ← targeted, cheap + + # Same loop written positionally rebuilds lhs every + # iteration even though it never changes: + for k in scenarios: + c.update(big_lhs_expr <= rhs_k) # ← avoid + Parameters ---------- constraint : ConstraintLike, optional From 70dbed4281d3c967f2a24560785bf769a5087ad6 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 25 May 2026 13:27:24 +0200 Subject: [PATCH 07/13] fix(persistent): default ModelDiff.from_snapshot(same_model=False) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the A1 residual from the #718 review. The flag-trust path (`skip_coef_compare = same_model and not coef_dirty`) is correct through Constraint.update() (set in one place, shims forward), but `c.coeffs.values[...] = ...` still bypasses _coef_dirty. With same_model=True as the default, that bypass silently produces wrong diffs. Flip the default to False. Cross-model paths (the only production caller, Solver._update_locked, passes explicitly) are unaffected. Same-model warm-update paths now value-diff the CSR data — small perf hit (50-200ms at Mayk-scale per Mayk's bench), correct by default. Solver-aware callers who own the mutation contract can opt back into the optimization with `same_model=True`. Co-Authored-By: Claude Opus 4.7 (1M context) --- linopy/persistent/diff.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/linopy/persistent/diff.py b/linopy/persistent/diff.py index 46a866f2..f100c75e 100644 --- a/linopy/persistent/diff.py +++ b/linopy/persistent/diff.py @@ -343,7 +343,7 @@ def from_snapshot( cls, snapshot: ModelSnapshot, model: Model, - same_model: bool = True, + same_model: bool = False, ignore_dims: Iterable[str] = (), ) -> ModelDiff: """ @@ -353,6 +353,14 @@ def from_snapshot( a mismatch triggers ``RebuildReason.COORD_REINDEX``. Pass ``ignore_dims={"snapshot"}`` for rolling-horizon use cases where the snapshot coord legitimately shifts between solves. + + ``same_model`` is a perf hint, **default False**. When True, the + diff trusts ``Constraint._coef_dirty`` to short-circuit the CSR + walk for unchanged containers (`skip_coef_compare`). That's only + safe if every coefficient mutation went through ``Constraint.update`` + (or the setters that forward there) — direct ``c.coeffs.values[...]`` + writes bypass the flag and would silently miss changes. Pass + ``same_model=True`` only when you own the mutation contract. """ ignored = frozenset(ignore_dims) check_coords = True From f2d69c487bd1dc97d78e05cb74db1ea0609e1402 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 25 May 2026 14:55:28 +0200 Subject: [PATCH 08/13] docs: teach .update() in tutorials; mark setters as syntactic sugar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - examples/manipulating-models.ipynb: rewrite mutation cells to use Variable.update / Constraint.update; setter form is mentioned in notes as syntactic sugar for the same call. - examples/creating-constraints.ipynb: reframe the CSRConstraint vs Constraint API table around .update() as the mutation API; setters are sugar. - Setter docstrings now say 'syntactic sugar for Constraint/Variable .update; do not add logic here so the contract stays single-sourced' — a directive to future contributors as much as to readers. No deprecation, no breaking change. .update() is the documented canonical mutation API; the seven setters continue to exist as one-line shims. Co-Authored-By: Claude Opus 4.7 (1M context) --- examples/creating-constraints.ipynb | 6 +++--- examples/manipulating-models.ipynb | 33 +++++++++++++++++++---------- linopy/constraints.py | 10 ++++----- linopy/variables.py | 10 +++++++-- 4 files changed, 38 insertions(+), 21 deletions(-) diff --git a/examples/creating-constraints.ipynb b/examples/creating-constraints.ipynb index 1b792b14..5070916f 100644 --- a/examples/creating-constraints.ipynb +++ b/examples/creating-constraints.ipynb @@ -348,7 +348,7 @@ "\n", "`CSRConstraint` deliberately exposes a narrower API than the xarray-backed `Constraint`:\n", "\n", - "- **No in-place mutation.** Setters such as `con.coeffs = ...`, `con.vars = ...`, `con.sign = ...`, `con.rhs = ...`, and `con.lhs = ...` are only available on `Constraint`.\n", + "- **No in-place mutation.** `Constraint.update(...)` (and its setter sugar — `con.coeffs = ...`, `con.vars = ...`, `con.sign = ...`, `con.rhs = ...`, `con.lhs = ...`) is only available on `Constraint`.\n", "- **No label-based indexing.** `con.loc[...]` is only available on `Constraint`.\n", "- **Accessing `.coeffs` / `.vars` triggers reconstruction.** On a `CSRConstraint` these properties rebuild the full xarray `Dataset` on demand and emit a `PerformanceWarning`. For solver-oriented workflows prefer `con.to_matrix()` or work with the CSR data directly.\n", "\n", @@ -356,8 +356,8 @@ "\n", "```python\n", "con = m.constraints[\"my_constraint\"].mutable()\n", - "con.loc[{\"time\": 0}] # label-based indexing now available\n", - "con.rhs = 5 # mutation now available\n", + "con.loc[{\"time\": 0}] # label-based indexing now available\n", + "con.update(rhs=5) # mutation now available\n", "```" ] }, diff --git a/examples/manipulating-models.ipynb b/examples/manipulating-models.ipynb index 6903386b..0eea5e45 100644 --- a/examples/manipulating-models.ipynb +++ b/examples/manipulating-models.ipynb @@ -74,7 +74,7 @@ "metadata": {}, "outputs": [], "source": [ - "x.lower = 1" + "x.update(lower=1)" ] }, { @@ -83,7 +83,8 @@ "metadata": {}, "source": [ ".. note::\n", - " The same could have been achieved by calling `m.variables.x.lower = 1`\n", + " The setter form ``x.lower = 1`` (or ``m.variables['x'].lower = 1``)\n", + " is syntactic sugar for the same call.\n", "\n", "Let's solve it again!" ] @@ -127,7 +128,7 @@ "metadata": {}, "outputs": [], "source": [ - "x.lower = xr.DataArray(range(10, 0, -1), coords=(time,))" + "x.update(lower=xr.DataArray(range(10, 0, -1), coords=(time,)))" ] }, { @@ -157,9 +158,12 @@ "source": [ "## Varying Constraints\n", "\n", - "A similar functionality is implemented for constraints. Here we can modify the left-hand-side, the sign and the right-hand-side.\n", + "A similar functionality is implemented for constraints. We use\n", + "``Constraint.update`` to change the left-hand-side, the sign,\n", + "and the right-hand-side.\n", "\n", - "Assume we want to relax the right-hand-side of the first constraint `con1` to `8 * factor`. This would translate to:" + "Assume we want to relax the right-hand-side of the first constraint\n", + "``con1`` to ``8 * factor``. This translates to:" ] }, { @@ -169,7 +173,7 @@ "metadata": {}, "outputs": [], "source": [ - "con1.rhs = 8 * factor" + "con1.update(rhs=8 * factor)" ] }, { @@ -178,7 +182,9 @@ "metadata": {}, "source": [ ".. note::\n", - " The same could have been achieved by calling `m.constraints.con1.rhs = 8 * factor`\n", + " The setter form ``con1.rhs = 8 * factor`` (or\n", + " ``m.constraints['con1'].rhs = 8 * factor``) is syntactic sugar\n", + " for the same call.\n", "\n", "Let's solve it again!" ] @@ -212,7 +218,7 @@ "metadata": {}, "outputs": [], "source": [ - "con1.lhs = 3 * x + 8 * y" + "con1.update(lhs=3 * x + 8 * y)" ] }, { @@ -221,9 +227,14 @@ "metadata": {}, "source": [ "**Note:**\n", - "The same could have been achieved by calling \n", - "```python \n", - "m.constraints['con1'].lhs = 3 * x + 8 * y\n", + "The setter form ``con1.lhs = 3 * x + 8 * y`` (or\n", + "``m.constraints['con1'].lhs = 3 * x + 8 * y``) is syntactic sugar\n", + "for the same call.\n", + "\n", + "Both forms also accept a full constraint expression:\n", + "\n", + "```python\n", + "con1.update(3 * x + 8 * y <= 8 * factor) # replaces lhs / sign / rhs at once\n", "```" ] }, diff --git a/linopy/constraints.py b/linopy/constraints.py index 066614ac..e423d33e 100644 --- a/linopy/constraints.py +++ b/linopy/constraints.py @@ -1120,7 +1120,7 @@ def coeffs(self) -> DataArray: @coeffs.setter def coeffs(self, value: ConstantLike) -> None: - """Shim — forwards to :meth:`Constraint.update`.""" + """Syntactic sugar for :meth:`Constraint.update`. Do not add logic here; mutate via ``update`` so the contract stays single-sourced.""" self.update(coeffs=value) @property @@ -1129,7 +1129,7 @@ def vars(self) -> DataArray: @vars.setter def vars(self, value: variables.Variable | DataArray) -> None: - """Shim — forwards to :meth:`Constraint.update`.""" + """Syntactic sugar for :meth:`Constraint.update`. Do not add logic here; mutate via ``update`` so the contract stays single-sourced.""" self.update(variables=value) @property @@ -1138,7 +1138,7 @@ def sign(self) -> DataArray: @sign.setter def sign(self, value: SignLike) -> None: - """Shim — forwards to :meth:`Constraint.update`.""" + """Syntactic sugar for :meth:`Constraint.update`. Do not add logic here; mutate via ``update`` so the contract stays single-sourced.""" self.update(sign=value) @property @@ -1147,7 +1147,7 @@ def rhs(self) -> DataArray: @rhs.setter def rhs(self, value: ExpressionLike | VariableLike | ConstantLike) -> None: - """Shim — forwards to :meth:`Constraint.update`.""" + """Syntactic sugar for :meth:`Constraint.update`. Do not add logic here; mutate via ``update`` so the contract stays single-sourced.""" self.update(rhs=value) @property @@ -1157,7 +1157,7 @@ def lhs(self) -> expressions.LinearExpression: @lhs.setter def lhs(self, value: ExpressionLike | VariableLike | ConstantLike) -> None: - """Shim — forwards to :meth:`Constraint.update`.""" + """Syntactic sugar for :meth:`Constraint.update`. Do not add logic here; mutate via ``update`` so the contract stays single-sourced.""" self.update(lhs=value) def _assign_lhs_expr( diff --git a/linopy/variables.py b/linopy/variables.py index 04a1714f..1c79da7b 100644 --- a/linopy/variables.py +++ b/linopy/variables.py @@ -891,7 +891,10 @@ def upper(self) -> DataArray: @upper.setter def upper(self, value: ConstantLike) -> None: - """Shim — forwards to :meth:`Variable.update`.""" + """ + Syntactic sugar for :meth:`Variable.update`. Do not add logic + here; mutate via ``update`` so the contract stays single-sourced. + """ self.update(upper=value) @property @@ -906,7 +909,10 @@ def lower(self) -> DataArray: @lower.setter def lower(self, value: ConstantLike) -> None: - """Shim — forwards to :meth:`Variable.update`.""" + """ + Syntactic sugar for :meth:`Variable.update`. Do not add logic + here; mutate via ``update`` so the contract stays single-sourced. + """ self.update(lower=value) def update( From 2efdf118275a35aead2fc5c7f4caf6b1f0556f10 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Tue, 26 May 2026 12:40:57 +0200 Subject: [PATCH 09/13] deprecate(update): warn on mutation setters; promote .update() in docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds DeprecationWarning to all seven mutation setters (Variable.lower, Variable.upper, Constraint.coeffs, Constraint.vars, Constraint.sign, Constraint.rhs, Constraint.lhs). Each setter still forwards to .update() so existing code keeps working; the warning points at the canonical API. Internal sanitize_zeros migrated off setters (the last linopy/ caller). api.rst gains Modification sections listing .update() for both Variable and Constraint; tutorial notes rewritten to teach .update() and flag setters as deprecated. Release note added. dual.setter / solution.setter untouched — result assignment, not mutation, different deprecation track. Co-Authored-By: Claude Opus 4.7 (1M context) --- doc/api.rst | 18 ++++++++++++++ doc/release_notes.rst | 1 + examples/creating-constraints.ipynb | 2 +- examples/manipulating-models.ipynb | 22 ++++++++++------- linopy/constraints.py | 37 ++++++++++++++++++++++++++--- linopy/variables.py | 12 ++++++++++ 6 files changed, 79 insertions(+), 13 deletions(-) diff --git a/doc/api.rst b/doc/api.rst index f0afc322..707ba610 100644 --- a/doc/api.rst +++ b/doc/api.rst @@ -136,9 +136,14 @@ Attributes Modification ------------ +``Variable.update`` is the canonical mutation API. The legacy ``lower`` / +``upper`` setters still forward to ``update`` but emit a +``DeprecationWarning`` and will be removed in a future release. + .. autosummary:: :toctree: generated/ + variables.Variable.update variables.Variable.fix variables.Variable.unfix variables.Variable.relax @@ -330,6 +335,19 @@ Structure constraints.Constraint.coeffs constraints.Constraint.vars +Modification +------------ + +``Constraint.update`` is the canonical mutation API. The legacy ``lhs`` / +``sign`` / ``rhs`` / ``coeffs`` / ``vars`` setters still forward to +``update`` but emit a ``DeprecationWarning`` and will be removed in a +future release. + +.. autosummary:: + :toctree: generated/ + + constraints.Constraint.update + Post-solve access ----------------- diff --git a/doc/release_notes.rst b/doc/release_notes.rst index edd4ed07..6165d5d5 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -55,6 +55,7 @@ Most users should keep calling ``model.solve(...)``. If you want more control, y **Deprecations** * ``Solver.solve_problem``, ``Solver.solve_problem_from_model``, and ``Solver.solve_problem_from_file`` still work but emit a ``DeprecationWarning``. Use ``Solver.from_name(...).solve()`` (or simply ``model.solve(...)``) instead. They will be removed in a future release. +* Mutation via assignment to ``Variable.lower`` / ``Variable.upper`` / ``Constraint.coeffs`` / ``Constraint.vars`` / ``Constraint.lhs`` / ``Constraint.sign`` / ``Constraint.rhs`` is deprecated and emits a ``DeprecationWarning``. Use ``Variable.update(...)`` / ``Constraint.update(...)`` instead — the canonical mutation API with one validation path and one place that flips the persistent-solver dirty flag. Read access to these properties is unchanged. The setters will be removed in a future release. **Bug Fixes** diff --git a/examples/creating-constraints.ipynb b/examples/creating-constraints.ipynb index 5070916f..d504deb3 100644 --- a/examples/creating-constraints.ipynb +++ b/examples/creating-constraints.ipynb @@ -348,7 +348,7 @@ "\n", "`CSRConstraint` deliberately exposes a narrower API than the xarray-backed `Constraint`:\n", "\n", - "- **No in-place mutation.** `Constraint.update(...)` (and its setter sugar — `con.coeffs = ...`, `con.vars = ...`, `con.sign = ...`, `con.rhs = ...`, `con.lhs = ...`) is only available on `Constraint`.\n", + "- **No in-place mutation.** `Constraint.update(...)` is only available on `Constraint`. (The legacy setters — `con.coeffs = ...`, `con.vars = ...`, `con.sign = ...`, `con.rhs = ...`, `con.lhs = ...` — still forward to `update` on `Constraint` but emit a `DeprecationWarning` and will be removed in a future release.)\n", "- **No label-based indexing.** `con.loc[...]` is only available on `Constraint`.\n", "- **Accessing `.coeffs` / `.vars` triggers reconstruction.** On a `CSRConstraint` these properties rebuild the full xarray `Dataset` on demand and emit a `PerformanceWarning`. For solver-oriented workflows prefer `con.to_matrix()` or work with the CSR data directly.\n", "\n", diff --git a/examples/manipulating-models.ipynb b/examples/manipulating-models.ipynb index 0eea5e45..eb1097ab 100644 --- a/examples/manipulating-models.ipynb +++ b/examples/manipulating-models.ipynb @@ -83,8 +83,10 @@ "metadata": {}, "source": [ ".. note::\n", - " The setter form ``x.lower = 1`` (or ``m.variables['x'].lower = 1``)\n", - " is syntactic sugar for the same call.\n", + " Assignment via the ``x.lower = 1`` setter still works but is\n", + " deprecated and will be removed in a future release. Use\n", + " ``Variable.update`` instead — it is the canonical mutation API\n", + " with a single validation path.\n", "\n", "Let's solve it again!" ] @@ -182,9 +184,10 @@ "metadata": {}, "source": [ ".. note::\n", - " The setter form ``con1.rhs = 8 * factor`` (or\n", - " ``m.constraints['con1'].rhs = 8 * factor``) is syntactic sugar\n", - " for the same call.\n", + " Assignment via the ``con1.rhs = 8 * factor`` setter still works\n", + " but is deprecated and will be removed in a future release. Use\n", + " ``Constraint.update`` instead — it is the canonical mutation API\n", + " with a single validation path.\n", "\n", "Let's solve it again!" ] @@ -227,11 +230,12 @@ "metadata": {}, "source": [ "**Note:**\n", - "The setter form ``con1.lhs = 3 * x + 8 * y`` (or\n", - "``m.constraints['con1'].lhs = 3 * x + 8 * y``) is syntactic sugar\n", - "for the same call.\n", + "Assignment via the ``con1.lhs = 3 * x + 8 * y`` setter still works\n", + "but is deprecated and will be removed in a future release. Use\n", + "``Constraint.update`` instead — it is the canonical mutation API\n", + "with a single validation path.\n", "\n", - "Both forms also accept a full constraint expression:\n", + "``Constraint.update`` also accepts a full constraint expression in one call:\n", "\n", "```python\n", "con1.update(3 * x + 8 * y <= 8 * factor) # replaces lhs / sign / rhs at once\n", diff --git a/linopy/constraints.py b/linopy/constraints.py index e423d33e..7e1214eb 100644 --- a/linopy/constraints.py +++ b/linopy/constraints.py @@ -1121,6 +1121,12 @@ def coeffs(self) -> DataArray: @coeffs.setter def coeffs(self, value: ConstantLike) -> None: """Syntactic sugar for :meth:`Constraint.update`. Do not add logic here; mutate via ``update`` so the contract stays single-sourced.""" + warn( + "Constraint.coeffs setter is deprecated and will be removed in a " + "future release; use Constraint.update(coeffs=...) instead.", + DeprecationWarning, + stacklevel=2, + ) self.update(coeffs=value) @property @@ -1130,6 +1136,12 @@ def vars(self) -> DataArray: @vars.setter def vars(self, value: variables.Variable | DataArray) -> None: """Syntactic sugar for :meth:`Constraint.update`. Do not add logic here; mutate via ``update`` so the contract stays single-sourced.""" + warn( + "Constraint.vars setter is deprecated and will be removed in a " + "future release; use Constraint.update(variables=...) instead.", + DeprecationWarning, + stacklevel=2, + ) self.update(variables=value) @property @@ -1139,6 +1151,12 @@ def sign(self) -> DataArray: @sign.setter def sign(self, value: SignLike) -> None: """Syntactic sugar for :meth:`Constraint.update`. Do not add logic here; mutate via ``update`` so the contract stays single-sourced.""" + warn( + "Constraint.sign setter is deprecated and will be removed in a " + "future release; use Constraint.update(sign=...) instead.", + DeprecationWarning, + stacklevel=2, + ) self.update(sign=value) @property @@ -1148,6 +1166,12 @@ def rhs(self) -> DataArray: @rhs.setter def rhs(self, value: ExpressionLike | VariableLike | ConstantLike) -> None: """Syntactic sugar for :meth:`Constraint.update`. Do not add logic here; mutate via ``update`` so the contract stays single-sourced.""" + warn( + "Constraint.rhs setter is deprecated and will be removed in a " + "future release; use Constraint.update(rhs=...) instead.", + DeprecationWarning, + stacklevel=2, + ) self.update(rhs=value) @property @@ -1158,6 +1182,12 @@ def lhs(self) -> expressions.LinearExpression: @lhs.setter def lhs(self, value: ExpressionLike | VariableLike | ConstantLike) -> None: """Syntactic sugar for :meth:`Constraint.update`. Do not add logic here; mutate via ``update`` so the contract stays single-sourced.""" + warn( + "Constraint.lhs setter is deprecated and will be removed in a " + "future release; use Constraint.update(lhs=...) instead.", + DeprecationWarning, + stacklevel=2, + ) self.update(lhs=value) def _assign_lhs_expr( @@ -1420,9 +1450,10 @@ def to_matrix_with_rhs( def sanitize_zeros(self) -> Constraint: """Remove terms with zero or near-zero coefficients.""" not_zero = abs(self.coeffs) > 1e-10 - self.vars = self.vars.where(not_zero, -1) - self.coeffs = self.coeffs.where(not_zero) - return self + return self.update( + variables=self.vars.where(not_zero, -1), + coeffs=self.coeffs.where(not_zero), + ) def sanitize_missings(self) -> Constraint: """Mask out rows where all variables are missing (-1).""" diff --git a/linopy/variables.py b/linopy/variables.py index 1c79da7b..0f0826b8 100644 --- a/linopy/variables.py +++ b/linopy/variables.py @@ -895,6 +895,12 @@ def upper(self, value: ConstantLike) -> None: Syntactic sugar for :meth:`Variable.update`. Do not add logic here; mutate via ``update`` so the contract stays single-sourced. """ + warn( + "Variable.upper setter is deprecated and will be removed in a " + "future release; use Variable.update(upper=...) instead.", + DeprecationWarning, + stacklevel=2, + ) self.update(upper=value) @property @@ -913,6 +919,12 @@ def lower(self, value: ConstantLike) -> None: Syntactic sugar for :meth:`Variable.update`. Do not add logic here; mutate via ``update`` so the contract stays single-sourced. """ + warn( + "Variable.lower setter is deprecated and will be removed in a " + "future release; use Variable.update(lower=...) instead.", + DeprecationWarning, + stacklevel=2, + ) self.update(lower=value) def update( From 1b65509be1f50564377e08a22c25cf7062cb066b Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Tue, 26 May 2026 12:59:44 +0200 Subject: [PATCH 10/13] test(update): edge-case coverage; document rhs-rearrangement invariant Constraint.update tests: lhs-only, coeffs-only (vars preserved), compound lhs+sign, mutually-exclusive lhs+coeffs and lhs+variables. Variable.update tests: upper-only, valid array bound. Migrate test_constraint_coef_dirty.py from the now-deprecated setters to .update(), exercising the canonical path; add positional-form and no-op cases. Net effect: same dirty-flag invariants, 7 fewer warnings per pytest run. Docs: Constraint.update rhs= gains a worked example showing the two forms (constant vs variable/expression). add_constraints rhs gets a matching note pointing at the linopy invariant so the rearrangement rule is documented at the creation site too. Co-Authored-By: Claude Opus 4.7 (1M context) --- linopy/constraints.py | 23 +++++++++++--- test/test_constraint.py | 48 ++++++++++++++++++++++++++++++ test/test_constraint_coef_dirty.py | 43 ++++++++++++++++++-------- test/test_variable.py | 15 ++++++++++ 4 files changed, 112 insertions(+), 17 deletions(-) diff --git a/linopy/constraints.py b/linopy/constraints.py index 7e1214eb..1dd14dd9 100644 --- a/linopy/constraints.py +++ b/linopy/constraints.py @@ -1253,10 +1253,25 @@ def update( with ``coeffs`` / ``variables``. Sets the internal ``_coef_dirty`` flag. rhs : ExpressionLike / VariableLike / ConstantLike, optional - New right-hand side. Variable / Expression rhs is rearranged - onto the lhs (matching ``add_constraints``): the residual is - subtracted from ``c.lhs`` and only the constant part lands - on ``c.rhs``. + New right-hand side. + + * Constant rhs (scalar, array, DataArray) → assigned directly + to ``c.rhs``; ``c.lhs`` is untouched. + * Variable / Expression rhs → rearranged onto the lhs to + preserve the invariant that ``c.rhs`` is constant-only, + matching ``add_constraints``. **This rewrites ``c.lhs``.** + + Example — the two calls below produce the same final state:: + + # Form A: explicit, only changes rhs + c.update(rhs=5) + + # Form B: rhs carries a variable, so lhs is rewritten too. + # Starting from `2*x <= 3`, this gives `2*x - y <= 5`: + c.update(rhs=y + 5) + + If you want the rewrite to be loud, use the positional form + (``c.update(2*x - y <= 5)``) which makes both sides explicit. sign : SignLike, optional New sign. One of ``"<=" / "==" / ">="`` (or their ``< > =`` aliases). diff --git a/test/test_constraint.py b/test/test_constraint.py index 5fb6d29a..27f2ff7f 100644 --- a/test/test_constraint.py +++ b/test/test_constraint.py @@ -487,6 +487,54 @@ def test_constraint_update_positional_rejects_non_constraint( mc.update("not a constraint") # type: ignore +def test_constraint_update_lhs_only( + mc: linopy.constraints.Constraint, x: linopy.Variable, y: linopy.Variable +) -> None: + """lhs= alone replaces the expression; rhs and sign untouched.""" + old_rhs = mc.rhs.copy() + old_sign = mc.sign.copy() + mc.update(lhs=5 * x + 7 * y) + assert (mc.rhs == old_rhs).all() + assert (mc.sign == old_sign).all() + assert mc.lhs.nterm == 2 + + +def test_constraint_update_coeffs_only_keeps_values( + mc: linopy.constraints.Constraint, +) -> None: + """coeffs= alone replaces the coef array element-wise; vars untouched.""" + old_vars = mc.vars.copy() + mc.update(coeffs=mc.coeffs * 10) + assert (mc.vars == old_vars).all() + # original was mc.lhs with leading coeff; *10 → all coeffs *10 + assert mc.coeffs.max() >= 10 + + +def test_constraint_update_lhs_and_sign_together( + mc: linopy.constraints.Constraint, x: linopy.Variable +) -> None: + """Compound updates compose: lhs replacement + sign flip in one call.""" + mc.update(lhs=2 * x, sign=EQUAL) + assert (mc.sign == EQUAL).all() + assert mc.lhs.nterm == 1 + + +def test_constraint_update_lhs_and_coeffs_rejected( + mc: linopy.constraints.Constraint, x: linopy.Variable +) -> None: + """lhs= (full replacement) and coeffs= (partial) are mutually exclusive.""" + with pytest.raises(TypeError, match="lhs.*coeffs.*variables"): + mc.update(lhs=2 * x, coeffs=mc.coeffs * 2) + + +def test_constraint_update_lhs_and_variables_rejected( + mc: linopy.constraints.Constraint, x: linopy.Variable +) -> None: + """lhs= (full replacement) and variables= (partial) are mutually exclusive.""" + with pytest.raises(TypeError, match="lhs.*coeffs.*variables"): + mc.update(lhs=2 * x, variables=mc.vars) + + def test_constraint_rhs_setter_with_variable( mc: linopy.constraints.Constraint, x: linopy.Variable ) -> None: diff --git a/test/test_constraint_coef_dirty.py b/test/test_constraint_coef_dirty.py index 682eb6d8..83b956b7 100644 --- a/test/test_constraint_coef_dirty.py +++ b/test/test_constraint_coef_dirty.py @@ -19,55 +19,72 @@ def test_initial_coef_dirty_false(m_with_c: tuple[Model, str]) -> None: assert m.constraints[name]._coef_dirty is False -def test_coeffs_setter_sets_dirty(m_with_c: tuple[Model, str]) -> None: +def test_update_coeffs_sets_dirty(m_with_c: tuple[Model, str]) -> None: m, name = m_with_c c = m.constraints[name] - c.coeffs = c.coeffs * 2 + c.update(coeffs=c.coeffs * 2) assert c._coef_dirty is True -def test_vars_setter_sets_dirty(m_with_c: tuple[Model, str]) -> None: +def test_update_variables_sets_dirty(m_with_c: tuple[Model, str]) -> None: m, name = m_with_c c = m.constraints[name] - c.vars = c.vars + c.update(variables=c.vars) assert c._coef_dirty is True -def test_lhs_setter_sets_dirty(m_with_c: tuple[Model, str]) -> None: +def test_update_lhs_sets_dirty(m_with_c: tuple[Model, str]) -> None: m, name = m_with_c c = m.constraints[name] x = m.variables["x"] - c.lhs = 3 * x + c.update(lhs=3 * x) assert c._coef_dirty is True -def test_pure_constant_rhs_short_circuits(m_with_c: tuple[Model, str]) -> None: +def test_update_pure_constant_rhs_short_circuits(m_with_c: tuple[Model, str]) -> None: m, name = m_with_c c = m.constraints[name] coeffs_buf = c.data["coeffs"].values vars_buf = c.data["vars"].values - c.rhs = 9 + c.update(rhs=9) assert c._coef_dirty is False assert c.data["coeffs"].values is coeffs_buf assert c.data["vars"].values is vars_buf -def test_rhs_with_variable_sets_dirty(m_with_c: tuple[Model, str]) -> None: +def test_update_rhs_with_variable_sets_dirty(m_with_c: tuple[Model, str]) -> None: m, name = m_with_c c = m.constraints[name] x = m.variables["x"] - c.rhs = x + 3 + c.update(rhs=x + 3) assert c._coef_dirty is True -def test_sign_setter_does_not_set_dirty(m_with_c: tuple[Model, str]) -> None: +def test_update_sign_does_not_set_dirty(m_with_c: tuple[Model, str]) -> None: m, name = m_with_c c = m.constraints[name] - c.sign = "<=" + c.update(sign="<=") assert c._coef_dirty is False def test_flag_persists_across_container_access(m_with_c: tuple[Model, str]) -> None: m, name = m_with_c - m.constraints[name].coeffs = m.constraints[name].coeffs * 2 + m.constraints[name].update(coeffs=m.constraints[name].coeffs * 2) assert m.constraints[name]._coef_dirty is True + + +def test_update_positional_constraint_sets_dirty(m_with_c: tuple[Model, str]) -> None: + """Positional ``c.update(expr <= rhs)`` rewrites lhs and must flip the flag.""" + m, name = m_with_c + c = m.constraints[name] + x = m.variables["x"] + c.update(4 * x >= 7) + assert c._coef_dirty is True + + +def test_update_noop_does_not_set_dirty(m_with_c: tuple[Model, str]) -> None: + """``c.update()`` with no args is a no-op and must not flip the flag.""" + m, name = m_with_c + c = m.constraints[name] + c.update() + assert c._coef_dirty is False diff --git a/test/test_variable.py b/test/test_variable.py index 8064703b..110cf31c 100644 --- a/test/test_variable.py +++ b/test/test_variable.py @@ -225,6 +225,21 @@ def test_variable_update_array_invalid_dim(x: linopy.Variable) -> None: x.update(lower=pd.Series(range(15, 25))) +def test_variable_update_upper_only(z: linopy.Variable) -> None: + """upper= alone changes upper; lower untouched.""" + old_lower = z.lower.copy() + z.update(upper=25) + assert (z.upper == 25).all() + assert (z.lower == old_lower).all() + + +def test_variable_update_with_array(x: linopy.Variable) -> None: + """Array bound that aligns on the variable's coord is accepted.""" + lower = pd.Series(range(10, 20), index=pd.RangeIndex(10, name="first")) + x.update(lower=lower) + np.testing.assert_array_equal(x.lower.values, lower.values) + + def test_variable_sum(x: linopy.Variable) -> None: res = x.sum() assert res.nterm == 10 From f045355d3d36a4e214648cbfea4533c48487d01a Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Wed, 27 May 2026 09:54:23 +0200 Subject: [PATCH 11/13] review(update): address inline feedback on #727 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Constraint._assign_lhs_expr → _assign_lhs (drop redundant suffix; the method already takes a LinearExpression, so the type was in the signature, not the name). - Add Constraint._assign_data(**fields) helper. Wraps the four ``self._data = assign_multiindex_safe(self.data, **kw)`` callsites inside update() (rhs / coeffs / vars / sign). Untouched: the same pattern in dual.setter, sanitize_missings, sanitize_infinities — those aren't update() and stay out of scope here. - Add types.CONSTANT_TYPES tuple, derived from ConstantLike via get_args so the two cannot drift. Variable.update bound validation flipped from negative (isinstance against a hand-rolled non_constant tuple) to positive (isinstance against CONSTANT_TYPES); drops a redundant in-function ``from linopy import expressions`` (the module-level import already covered it). Co-Authored-By: Claude Opus 4.7 (1M context) --- linopy/constraints.py | 18 +++++++++++------- linopy/types.py | 3 ++- linopy/variables.py | 11 ++--------- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/linopy/constraints.py b/linopy/constraints.py index 1dd14dd9..43c0afcc 100644 --- a/linopy/constraints.py +++ b/linopy/constraints.py @@ -1190,7 +1190,7 @@ def lhs(self, value: ExpressionLike | VariableLike | ConstantLike) -> None: ) self.update(lhs=value) - def _assign_lhs_expr( + def _assign_lhs( self, expr: expressions.LinearExpression, rhs: DataArray | None = None ) -> None: """ @@ -1205,6 +1205,10 @@ def _assign_lhs_expr( ) self._coef_dirty = True + def _assign_data(self, **fields: Any) -> None: + """Internal: write ``fields`` into ``self._data`` via ``assign_multiindex_safe``.""" + self._data = assign_multiindex_safe(self.data, **fields) + def update( self, constraint: ConstraintLike | None = None, @@ -1319,7 +1323,7 @@ def update( # 1. lhs replacement first so subsequent rhs= rearrangement sees the new lhs. if lhs is not None: - self._assign_lhs_expr( + self._assign_lhs( expressions.as_expression( lhs, self.model, coords=self.coords, dims=self.coord_dims ) @@ -1332,16 +1336,16 @@ def update( ) residual = expr.reset_const() if residual.nterm != 0: - self._assign_lhs_expr(self.lhs - residual, rhs=expr.const) + self._assign_lhs(self.lhs - residual, rhs=expr.const) else: - self._data = assign_multiindex_safe(self.data, rhs=expr.const) + self._assign_data(rhs=expr.const) # 3. coeffs / variables partial updates (only valid without lhs=). if coeffs is not None: new_coeffs = DataArray(coeffs).broadcast_like( self.vars, exclude=[self.term_dim] ) - self._data = assign_multiindex_safe(self.data, coeffs=new_coeffs) + self._assign_data(coeffs=new_coeffs) self._coef_dirty = True if variables is not None: from linopy.variables import Variable as _Variable @@ -1353,13 +1357,13 @@ def update( f"Variable; got {type(variables).__name__}." ) new_vars = v.broadcast_like(self.coeffs, exclude=[self.term_dim]) - self._data = assign_multiindex_safe(self.data, vars=new_vars) + self._assign_data(vars=new_vars) self._coef_dirty = True # 4. sign last so it composes cleanly with the rest. if sign is not None: new_sign = maybe_replace_signs(DataArray(sign)).broadcast_like(self.sign) - self._data = assign_multiindex_safe(self.data, sign=new_sign) + self._assign_data(sign=new_sign) return self diff --git a/linopy/types.py b/linopy/types.py index 703c0a3b..aca72082 100644 --- a/linopy/types.py +++ b/linopy/types.py @@ -2,7 +2,7 @@ from collections.abc import Hashable, Iterable, Mapping, Sequence from pathlib import Path -from typing import TYPE_CHECKING, TypeAlias, Union +from typing import TYPE_CHECKING, TypeAlias, Union, get_args import numpy import polars as pl @@ -41,6 +41,7 @@ | DataFrame | pl.Series ) +CONSTANT_TYPES: tuple[type, ...] = get_args(ConstantLike) SignLike: TypeAlias = str | numpy.ndarray | DataArray | Series | DataFrame MaskLike: TypeAlias = numpy.ndarray | DataArray | Series | DataFrame PathLike: TypeAlias = str | Path diff --git a/linopy/variables.py b/linopy/variables.py index 0f0826b8..34bca25c 100644 --- a/linopy/variables.py +++ b/linopy/variables.py @@ -62,6 +62,7 @@ TERM_DIM, ) from linopy.types import ( + CONSTANT_TYPES, ConstantLike, DimsLike, ExpressionLike, @@ -967,16 +968,8 @@ def update( if lower is None and upper is None: return self - from linopy import expressions - - non_constant = ( - Variable, - ScalarVariable, - expressions.LinearExpression, - expressions.QuadraticExpression, - ) for name, val in (("lower", lower), ("upper", upper)): - if val is not None and isinstance(val, non_constant): + if val is not None and not isinstance(val, CONSTANT_TYPES): raise TypeError( f"Variable.update({name}=...) must be a constant; " f"got {type(val).__name__}." From 0b8748020b5b2a6a74f6fddb25308130150aab5d Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Wed, 27 May 2026 11:30:31 +0200 Subject: [PATCH 12/13] deprecate(update): FutureWarning on DataArray as variables=; extract _validate_update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Constraint - sanitize_zeros now writes _data via _assign_data directly (no longer round-trips through update(variables=DataArray), which would self-trigger the new deprecation warning). - Constraint.update(variables=...) emits FutureWarning when passed a raw DataArray of integer labels; Variable is the supported input. The path stays accepted for back-compat and will be removed alongside the v1 cleanup. Variable - Extract Variable._validate_update(*, lower, upper) — validates, broadcasts, and runs the cross-bound (lb<=ub) check, returning a dict ready for assignment. update() body shrinks to ~3 lines. Coord validation (parity with add_variables) deferred to #726 land. Tests - test_constraint_coef_dirty's variables= test now passes a Variable instead of a DataArray (matches the supported input). - test_constraint_vars_setter_with_array wrapped in pytest.warns(FutureWarning) — locks in the deprecation contract. Co-Authored-By: Claude Opus 4.7 (1M context) --- doc/release_notes.rst | 1 + linopy/constraints.py | 33 +++++++++++++++------ linopy/variables.py | 46 +++++++++++++++++++----------- test/test_constraint.py | 4 ++- test/test_constraint_coef_dirty.py | 3 +- 5 files changed, 59 insertions(+), 28 deletions(-) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index 6165d5d5..1af71356 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -56,6 +56,7 @@ Most users should keep calling ``model.solve(...)``. If you want more control, y * ``Solver.solve_problem``, ``Solver.solve_problem_from_model``, and ``Solver.solve_problem_from_file`` still work but emit a ``DeprecationWarning``. Use ``Solver.from_name(...).solve()`` (or simply ``model.solve(...)``) instead. They will be removed in a future release. * Mutation via assignment to ``Variable.lower`` / ``Variable.upper`` / ``Constraint.coeffs`` / ``Constraint.vars`` / ``Constraint.lhs`` / ``Constraint.sign`` / ``Constraint.rhs`` is deprecated and emits a ``DeprecationWarning``. Use ``Variable.update(...)`` / ``Constraint.update(...)`` instead — the canonical mutation API with one validation path and one place that flips the persistent-solver dirty flag. Read access to these properties is unchanged. The setters will be removed in a future release. +* Passing a raw ``DataArray`` of integer labels to ``Constraint.vars = ...`` setter is deprecated and emits a ``FutureWarning``. Pass a ``Variable`` to ``Constraint.update()`` instead — it is the supported input. The ``DataArray`` path will be removed in a future release. **Bug Fixes** diff --git a/linopy/constraints.py b/linopy/constraints.py index 43c0afcc..57e04870 100644 --- a/linopy/constraints.py +++ b/linopy/constraints.py @@ -1282,11 +1282,14 @@ def update( coeffs : ConstantLike, optional Replace coefficient values (same sparsity / term structure). Lower-level than ``lhs=``; sets ``_coef_dirty``. - variables : Variable / DataArray, optional + variables : Variable, optional Replace variable label array (same sparsity / term structure). Lower-level than ``lhs=``; sets ``_coef_dirty``. - Mirrors the ``c.vars`` attribute; spelled out here to avoid - shadowing Python's ``vars()`` builtin in the kwarg name. + + A raw ``DataArray`` of integer labels is still accepted + for back-compat but emits a ``FutureWarning`` — pass a + ``Variable`` instead. The DataArray path will be removed + in a future release. Returns ------- @@ -1350,11 +1353,21 @@ def update( if variables is not None: from linopy.variables import Variable as _Variable - v = variables.labels if isinstance(variables, _Variable) else variables - if not isinstance(v, DataArray): + if isinstance(variables, _Variable): + v = variables.labels + elif isinstance(variables, DataArray): + warnings.warn( + "Passing a DataArray to Constraint.update(variables=...) " + "is deprecated and will be removed in a future release; " + "pass a Variable instead.", + FutureWarning, + stacklevel=2, + ) + v = variables + else: raise TypeError( - "Constraint.update(variables=...) expects a DataArray or " - f"Variable; got {type(variables).__name__}." + "Constraint.update(variables=...) expects a Variable; " + f"got {type(variables).__name__}." ) new_vars = v.broadcast_like(self.coeffs, exclude=[self.term_dim]) self._assign_data(vars=new_vars) @@ -1469,10 +1482,12 @@ def to_matrix_with_rhs( def sanitize_zeros(self) -> Constraint: """Remove terms with zero or near-zero coefficients.""" not_zero = abs(self.coeffs) > 1e-10 - return self.update( - variables=self.vars.where(not_zero, -1), + self._assign_data( + vars=self.vars.where(not_zero, -1), coeffs=self.coeffs.where(not_zero), ) + self._coef_dirty = True + return self def sanitize_missings(self) -> Constraint: """Mask out rows where all variables are missing (-1).""" diff --git a/linopy/variables.py b/linopy/variables.py index 34bca25c..ae60a33f 100644 --- a/linopy/variables.py +++ b/linopy/variables.py @@ -968,25 +968,39 @@ def update( if lower is None and upper is None: return self - for name, val in (("lower", lower), ("upper", upper)): - if val is not None and not isinstance(val, CONSTANT_TYPES): + updates = self._validate_update(lower=lower, upper=upper) + self._data = assign_multiindex_safe(self.data, **updates) + return self + + def _validate_update( + self, + *, + lower: ConstantLike | None = None, + upper: ConstantLike | None = None, + ) -> dict[str, DataArray]: + """ + Validate, broadcast, and cross-check update inputs. + + Returns the broadcasted DataArrays ready for assignment. Raises + before any mutation if any input is wrong. + """ + updates: dict[str, DataArray] = {} + own_dims = self.model.variables[self.name].dims + for name, val, ref in ( + ("lower", lower, self.lower), + ("upper", upper, self.upper), + ): + if val is None: + continue + if not isinstance(val, CONSTANT_TYPES): raise TypeError( f"Variable.update({name}=...) must be a constant; " f"got {type(val).__name__}." ) - - updates: dict[str, DataArray] = {} - own_dims = self.model.variables[self.name].dims - if lower is not None: - new_lower = DataArray(lower).broadcast_like(self.lower) - if not set(new_lower.dims).issubset(own_dims): - raise ValueError("Cannot assign new dimensions to existing variable.") - updates["lower"] = new_lower - if upper is not None: - new_upper = DataArray(upper).broadcast_like(self.upper) - if not set(new_upper.dims).issubset(own_dims): + new_val = DataArray(val).broadcast_like(ref) + if not set(new_val.dims).issubset(own_dims): raise ValueError("Cannot assign new dimensions to existing variable.") - updates["upper"] = new_upper + updates[name] = new_val final_lower = updates.get("lower", self.lower) final_upper = updates.get("upper", self.upper) @@ -994,9 +1008,7 @@ def update( raise ValueError( "Variable.update would leave lower > upper at one or more coordinates." ) - - self._data = assign_multiindex_safe(self.data, **updates) - return self + return updates @property @has_optimized_model diff --git a/test/test_constraint.py b/test/test_constraint.py index 27f2ff7f..a6e96117 100644 --- a/test/test_constraint.py +++ b/test/test_constraint.py @@ -357,7 +357,9 @@ def test_constraint_vars_setter( def test_constraint_vars_setter_with_array( mc: linopy.constraints.Constraint, x: linopy.Variable ) -> None: - mc.vars = x.labels + """Passing a raw DataArray is deprecated but still works for back-compat.""" + with pytest.warns(FutureWarning, match="DataArray"): + mc.vars = x.labels assert_equal(mc.vars, x.labels) diff --git a/test/test_constraint_coef_dirty.py b/test/test_constraint_coef_dirty.py index 83b956b7..6e32217b 100644 --- a/test/test_constraint_coef_dirty.py +++ b/test/test_constraint_coef_dirty.py @@ -29,7 +29,8 @@ def test_update_coeffs_sets_dirty(m_with_c: tuple[Model, str]) -> None: def test_update_variables_sets_dirty(m_with_c: tuple[Model, str]) -> None: m, name = m_with_c c = m.constraints[name] - c.update(variables=c.vars) + x = m.variables["x"] + c.update(variables=x) assert c._coef_dirty is True From ae11fdcc670e2a60ab6934e8558b099cd979198c Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Wed, 27 May 2026 11:43:55 +0200 Subject: [PATCH 13/13] =?UTF-8?q?refactor(constraints):=20=5Fassign=5Fdata?= =?UTF-8?q?=20=E2=86=92=20=5Fupdate=5Fdata;=20manage=20=5Fcoef=5Fdirty=20i?= =?UTF-8?q?nside?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The helper now writes fields AND flips _coef_dirty when the written set includes coeffs or vars. Callsites in update() (coeffs / vars branches) and sanitize_zeros drop their explicit `self._coef_dirty = True` lines — the rule lives in one place, can't be forgotten by future field additions. rhs / sign writes still don't dirty (correctly). _assign_lhs is untouched: it uses a different write mechanic (drop_vars + assign) and manages its own flag. Co-Authored-By: Claude Opus 4.7 (1M context) --- linopy/constraints.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/linopy/constraints.py b/linopy/constraints.py index 57e04870..bf963e0c 100644 --- a/linopy/constraints.py +++ b/linopy/constraints.py @@ -1205,9 +1205,16 @@ def _assign_lhs( ) self._coef_dirty = True - def _assign_data(self, **fields: Any) -> None: - """Internal: write ``fields`` into ``self._data`` via ``assign_multiindex_safe``.""" + def _update_data(self, **fields: Any) -> None: + """ + Internal: write ``fields`` into ``self._data`` and update dirty bookkeeping. + + Writes that touch the lhs structure (``coeffs``, ``vars``) flip + ``_coef_dirty``. Other fields (``rhs``, ``sign``, …) leave it alone. + """ self._data = assign_multiindex_safe(self.data, **fields) + if "coeffs" in fields or "vars" in fields: + self._coef_dirty = True def update( self, @@ -1341,15 +1348,14 @@ def update( if residual.nterm != 0: self._assign_lhs(self.lhs - residual, rhs=expr.const) else: - self._assign_data(rhs=expr.const) + self._update_data(rhs=expr.const) # 3. coeffs / variables partial updates (only valid without lhs=). if coeffs is not None: new_coeffs = DataArray(coeffs).broadcast_like( self.vars, exclude=[self.term_dim] ) - self._assign_data(coeffs=new_coeffs) - self._coef_dirty = True + self._update_data(coeffs=new_coeffs) if variables is not None: from linopy.variables import Variable as _Variable @@ -1370,13 +1376,12 @@ def update( f"got {type(variables).__name__}." ) new_vars = v.broadcast_like(self.coeffs, exclude=[self.term_dim]) - self._assign_data(vars=new_vars) - self._coef_dirty = True + self._update_data(vars=new_vars) # 4. sign last so it composes cleanly with the rest. if sign is not None: new_sign = maybe_replace_signs(DataArray(sign)).broadcast_like(self.sign) - self._assign_data(sign=new_sign) + self._update_data(sign=new_sign) return self @@ -1482,11 +1487,10 @@ def to_matrix_with_rhs( def sanitize_zeros(self) -> Constraint: """Remove terms with zero or near-zero coefficients.""" not_zero = abs(self.coeffs) > 1e-10 - self._assign_data( + self._update_data( vars=self.vars.where(not_zero, -1), coeffs=self.coeffs.where(not_zero), ) - self._coef_dirty = True return self def sanitize_missings(self) -> Constraint: