Skip to content

fix: normalize alternative constraint signs on construction - #1088

Merged
FabianHofmann merged 3 commits into
masterfrom
fix/1054-to-constraint-sign
Oct 30, 2026
Merged

FabianHofmann merged 3 commits into
masterfrom
fix/1054-to-constraint-sign

Conversation

@FabianHofmann

Copy link
Copy Markdown
Collaborator

Closes #1054.

Note

The following content was generated by AI.

Changes proposed in this Pull Request

LinearExpression.to_constraint("==", rhs) kept the sign "==" instead of the canonical "=". As a result, repr() of the constraint raised KeyError: '==' and the wrong sign was stored in the model. On current master, this happens on both the dense and the sparse backend.

The cause: signs were only normalized when add_constraints received a separate sign argument. Constructing a Constraint itself never normalized them.

  • Constraint._bind now normalizes the sign, so every construction path (to_constraint, from_dataset, adding a constraint to a model) stores canonical signs.
  • maybe_replace_signs is vectorized because it now runs on every constraint construction. It only converts the distinct non-canonical values and still raises ValueError on invalid signs.
  • A new allow_null flag lets _bind keep NaN signs on absent rows, which shift/reindex produce. User-facing paths in model.py keep the strict default, so a NaN sign passed by the user still raises.
  • New tests cover "==", "<" and ">" through to_constraint on dense and sparse (sign, repr, stored sign), and through Constraint.from_dataset.

Checklist

  • AI-generated content is marked (see AGENTS.md).
  • Code changes are sufficiently documented; i.e. new functions contain docstrings and further explanations may be given in doc.
  • Unit tests for new features were added (if applicable).
  • A note for the release notes doc/release_notes.rst of the upcoming release is included.
  • I consent to the release of this PR's code under the MIT license.

Constraint._bind now maps '==', '<', '>' to canonical signs, so
to_constraint('==') no longer breaks repr or stores '==' in the model.
maybe_replace_signs is vectorized and keeps null signs of absent rows.
@codspeed

codspeed Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 181 untouched benchmarks
⏩ 181 skipped benchmarks1


Comparing fix/1054-to-constraint-sign (d069964) with master (185c001)

Open in CodSpeed

Footnotes

  1. 181 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@FabianHofmann
FabianHofmann merged commit a51e4ff into master Oct 30, 2026
22 checks passed
@FabianHofmann
FabianHofmann deleted the fix/1054-to-constraint-sign branch October 30, 2026 20:08
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.

Dense LinearExpression.to_constraint('==') keeps sign '==': repr raises KeyError, differs from sparse '='

1 participant