Skip to content

Annotate arithmetic operations#1231

Open
jonathanberthias wants to merge 36 commits into
scipopt:masterfrom
jonathanberthias:type-expr-ops
Open

Annotate arithmetic operations#1231
jonathanberthias wants to merge 36 commits into
scipopt:masterfrom
jonathanberthias:type-expr-ops

Conversation

@jonathanberthias

Copy link
Copy Markdown
Contributor

As a follow up to #1229, we can now quickly evaluate the effect of annotating all the arithmetic operations.
This PR adds annotations to the stubs for nearly all the simple cases. There are still some areas where the annotations are not correct, and they fall broadly in the following categories:

  1. overloading np.ndarray methods: we are changing the return values of the operations compared to the base class, which in the best case works with a type: ignore[override] comment, and in the worst case makes it impossible to get the type checker to infer the correct result
  2. outputs that depend on the number of dimensions: for instance, matrix multiplication between two MatrixExpr returns a MatrixExpr in general, except if both are 1D in which case the result is an Expr. Likewise, operations with 0D-MatrixExpr fail to type-check correctly. This kind of "shape-typing" is possible to do I believe, but is more complex than I reached for in this PR
  3. operations with Decimal: I annotated things with float to keep them simple, but that is actually too restrictive. Semantically typing.SupportsFloat is correct, but that also matches np.ndarray which makes many operations fail to type-check. This is an open issue to be solved
  4. operations with Term: somehow many operations with a Term object succeed at runtime, but that seems wrong to me
  5. all inplace operations: Mypy does not handle cases where the type of a variable changes after an inplace binary operation. Other type checkers (Pyright, Ty, Pyrefly) do handle this case correctly though, so we might want to add more checkers to the test suite.

Part of #1072

@jonathanberthias jonathanberthias marked this pull request as ready for review July 4, 2026 22:21
- name: Install Ruff
uses: astral-sh/ruff-action@v3
with:
version: "0.14.11"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the pin because this version was causing a weird formatting that was fixed in recent versions, but feel free to pin to a higher version if you want to keep a pinned version @Joao-Dionisio

@Joao-Dionisio

Joao-Dionisio commented Jul 5, 2026

Copy link
Copy Markdown
Member

Hey @jonathanberthias , thank you, great job on greatly reducing the number of errors! Here's what my Claude flagged:

1. from typing import Self breaks type-checking on Python < 3.11 targets

Typeshed only exports Self from typing on 3.11+, and since we declare requires-python = ">=3.8" and ship py.typed, anyone running mypy with --python-version 3.10 or older gets an error inside scip.pyi itself. The file already imports from typing_extensions, so it's a one-word move:

-from typing import Any, AnyStr, ClassVar, Literal, Self, SupportsFloat, Union, overload
+from typing import Any, AnyStr, ClassVar, Literal, SupportsFloat, Union, overload

-from typing_extensions import CapsuleType, disjoint_base
+from typing_extensions import CapsuleType, Self, disjoint_base

2. Module-level exp/log/sqrt/sin/cos are narrower than the runtime

_wrap_ufunc (and the docstrings) also accept numbers, lists, tuples, and plain np.ndarray, so e.g. sqrt(2.0) or exp(np.array([...])) now fail to type-check even though they work. Two extra overloads would cover it:

@overload
def exp(x: float) -> UnaryExpr: ...
@overload
def exp(x: np.ndarray | list | tuple) -> MatrixGenExpr: ...

Fine as a follow-up too.

3. Expr.__pow__ is value-dependent

The runtime branches on the value of the exponent (non-negative integer-valued → Expr, otherwise → PowExpr), so x ** -1 type-checks as Expr but is a PowExpr at runtime, and x ** 2.0 is the reverse. I know this can't be expressed statically — maybe just add a short comment in the stub so nobody tries to "fix" it later.

4. Naming nits in the __pow__/__rpow__ signatures

The second Expr.__pow__ overload has module instead of modulo. The file also now mixes modulo and mod, and Expr.__rpow__ has no third parameter while GenExpr.__rpow__ does — might be worth unifying while you're at it.

5. stubs/baseline.sh

What problem does the /tmp indirection solve? If it's about not truncating the baseline before mypy finishes, tmp=$(mktemp) + mv would be atomic and avoid collisions between concurrent runs / leftover files.

6. Tiny nit

Expr.__ne__ was removed (👍 since the runtime raises), but GenExpr.__ne__ -> bool is still there even though the runtime raises for it as well — could be dropped for symmetry.

@Joao-Dionisio

Copy link
Copy Markdown
Member

By the way, @jonathanberthias , apologies if I've already asked this, but what are you using SCIP for, out of curiosity? 😄

@jonathanberthias

Copy link
Copy Markdown
Contributor Author

Thanks for the review, all good findings!

By the way, @jonathanberthias , apologies if I've already asked this, but what are you using SCIP for, out of curiosity? 😄

At my previous job, I was using it for portfolio optimization with many integer constraints as an alternative to Gurobi, and I was very impressed with the quality of the results! I don't use it that much anymore though, it's just a side-project 😄

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.

2 participants