Tighten type hints on add_variables / add_constraints / add_objective
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 257
- Forks
- 87
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 29
Description
While reviewing linopy/model.py, several typing improvements stood out on the main Model.add_* entry points. One is a latent correctness bug in the overloads; the rest are consistency / precision fixes that take advantage of aliases that already live in linopy/types.py.
1. add_constraints overload bug (correctness)
Model.add_constraints has overloads for freeze: Literal[False] = ... and freeze: Literal[True] = ... (model.py:884–913), but the runtime default is freeze: bool | None = None (model.py:927). None is the sentinel that means "use self.freeze_constraints".
Consequences:
- A caller passing
freeze=Noneexplicitly matches neither overload. - The overloads claim the default is
Literal[False], which is wrong: whenself.freeze_constraints=Trueand the caller omitsfreeze, mypy infersConstraintbut the runtime returnsCSRConstraint.
Fix: add a third overload for freeze: None = ... (or no-arg) returning ConstraintBase, and drop the misleading defaults from the two typed-bool overloads (use :, not = ...).
2. add_variables uses bespoke types where aliases exist (model.py:656)
lower: Any = -inf,upper: Any = inf→ should beConstantLike. This matches whatas_dataarrayactually accepts at model.py:774–775, andAnydefeats type-checking on the most common entry point.coords: Sequence[Sequence | pd.Index | DataArray] | Mapping | None→CoordsLike | None. The inline form is also slightly narrower — it omitsDataArrayCoordinates/DatasetCoordinates.mask: DataArray | ndarray | Series | None→MaskLike | None. The current inline form dropsDataFrameeven thoughas_dataarrayaccepts it (used at model.py:784).
3. add_constraints runtime signature (model.py:915)
coords: Sequence[Sequence | pd.Index | DataArray] | Mapping | None should be CoordsLike | None, matching the overloads and as_dataarray.
4. add_objective (model.py:1093)
sense: str = \"min\"→Literal[\"min\", \"max\"]. The setter atobjective.py:216–217raises on anything else, so the widerstris just hiding a runtime error from the type-checker.exprincludesSequence[tuple[ConstantLike, VariableLike]]but the body at model.py:1122–1124 only handlesVariable | LinearExpression | QuadraticExpression. Either the type is too wide or the body is missing a branch — worth verifying whether the tuple-sequence path is reachable or dead.
Lower-priority
add_constraintslhs: ... | Callable—Callableis unparameterized. Tightening toCallable[..., ConstraintLike | AnonymousScalarConstraint]would document the rule contract but may break existing callers that return raw expressions/tuples.- Consider introducing a
BoundLike = ConstantLikealias for symmetry — small, but it documents intent atadd_variablesbetter than reusingConstantLike.
Suggested order
- Fix
add_constraintsfreezeoverloads (correctness, not just hygiene). - Tighten
add_objective.sensetoLiteral[\"min\", \"max\"]. - Replace inline
coords/mask/ bound types withCoordsLike/MaskLike/ConstantLikeacross the three functions. - Audit whether
Sequence[tuple[ConstantLike, VariableLike]]inadd_objectiveis reachable or dead.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in linopy/model.py at the add_variables, add_constraints overloads and runtime signature, and add_objective; read the aliases in linopy/types.py and the sense validation in objective.py:216–217. Verify the tuple-sequence path around model.py:1122–1124, then update the annotations so aliases, freeze=None, and valid objective senses are represented accurately and type checking passes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend-api-design
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100