Return `NotImplemented` for `Expr` and `GenExpr` operators to simplify by Zeroto521 · Pull Request #1182 · scipopt/PySCIPOpt

Zeroto521 · 2026-01-31T14:40:52Z

Python prefers to use NotImplemented to handle the unknown input types. It's clear.

Here is an example.
A.__add__ can only receive int type. But a + b can work. When Python calls A.__add__(B), it returns NotImplemented. Then Python will call B.__radd__(A). If both of them are NotImplemented, Python will raise a TypeError.

So we use NotImplemented to simplify Expr, GenExpr, and MatrixExpr.

from __future__ import annotations


class A:
    def __init__(self, value: int):
        self.value = value

    def __add__(self, other: int) -> int:
        if not isinstance(other, int):
            return NotImplemented
        return self.value + other


class B:
    def __init__(self, value: int):
        self.value = value

    def __add__(self, other: A) -> B:
        if not isinstance(other, A):
            return NotImplemented
        return self.value + other.value

    def __radd__(self, other: A) -> int:
        if not isinstance(other, A):
            return NotImplemented
        return self.value + other.value


if __name__ == "__main__":
    a = A(5)
    b = B(10)

    print(a + 3)  # print 8
    print(a + b)  # print 15
    print(a + "test")  # raises TypeError
# Traceback (most recent call last):
#   File "test.py", line 35, in <module>
#     print(a + "test")  # raises TypeError
#           ~~^~~~~~~~
# TypeError: unsupported operand type(s) for +: 'A' and 'str'

Improves the _expr_richcmp function by using explicit type checks, handling numpy arrays and numbers more robustly, and leveraging Python C API comparison constants. This refactor enhances error handling and code readability, and ensures unsupported types raise clear exceptions.

Replaces repeated isinstance checks for numeric types with a shared NUMBER_TYPES tuple. This improves maintainability and consistency in type checking within _expr_richcmp and related code.

Modified __mul__ and __add__ methods in Expr and GenExpr classes to return NotImplemented when the operand is not an instance of EXPR_OP_TYPES, improving type safety and operator behavior.

Replaces the explicit type tuple with EXPR_OP_TYPES in the type check for 'other' in _expr_richcmp, raising TypeError for unsupported types. This improves maintainability and consistency in type validation.

Enhanced type validation in expression operator overloads by returning NotImplemented for unsupported types and raising TypeError in buildGenExprObj for invalid inputs. Removed special handling for numpy arrays and simplified code paths for better maintainability and clearer error reporting.

Simplified the implementation of __truediv__ and __rtruediv__ by directly using the division operator instead of calling __truediv__ explicitly on generated expression objects.

float(Expr) can't return True

Replaces manual timing with the timeit module for more accurate performance measurement in matrix operation tests. Updates assertions to require the optimized implementation to be at least 25% faster, and reduces test parameterization to n=100 for consistency.

Replaces random matrix generation with a stacked matrix of zeros and ones in test_matrix_dot_performance to provide more controlled test data.

Replaces the custom _is_number function with isinstance checks against NUMBER_TYPES for improved clarity and consistency in type checking throughout expression operations.

Simplifies type checking and arithmetic operations in Expr and GenExpr classes by removing unnecessary float conversions and reordering type checks. Also improves error handling for exponentiation and constraint comparisons.

Documented that Expr and GenExpr now return NotImplemented when they cannot handle other types in calculations.

Replaces EXPR_OP_TYPES with GENEXPR_OP_TYPES in GenExpr and related functions to distinguish between Expr and GenExpr operations. Removes special handling for GenExpr in Expr arithmetic methods, simplifying type logic and improving consistency.

Clarified the changelog note to specify that NotImplemented is returned for Expr and GenExpr operators when they can't handle input types in calculations.

Corrects error messages in Expr and GenExpr exponentiation to display the correct variable. Removes an unnecessary assertion in Model and replaces a call to _is_number with isinstance for type checking in readStatistics.

Replaces EXPR_OP_TYPES with GENEXPR_OP_TYPES in the type check to ensure correct type validation in the _expr_richcmp method.

src/pyscipopt/expr.pxi

Casts the exponent base to float in GenExpr's __pow__ method to prevent type errors when reformulating expressions using log and exp.

Explicitly converts 'other' to float in the exponentiation method to avoid type errors when using non-float numeric types.

The _is_number symbol was removed from the list of incomplete stubs in scip.pyi, likely because it is no longer needed or has been implemented.

Ensures that multiplication of Expr by numeric types consistently uses float conversion, preventing potential type errors when multiplying terms.

Zeroto521 · 2026-04-01T14:05:12Z

mypy-1.20.0 causes CI to fail. mypy-1.19.0 could pass.

error: pyscipopt.scip.Column.__eq__ is inconsistent, stub parameter "other" should be positional-only (add "/", e.g. "value, /")
Stub: in file /opt/hostedtoolcache/Python/3.14.3/x64/lib/python3.14/site-packages/pyscipopt/scip.pyi:123
def (self: pyscipopt.scip.Column, other: object) -> bool
Runtime:
def (self, value, /)

Replace Python-level float(other) calls with Cython <double>other casts in _expr_richcmp when `other` is numeric. This updates the lhs/rhs arguments for ExprCons in the <=, >=, and == branches to use direct C double values, reducing Python overhead while preserving behavior.

Update _expr_richcmp in src/pyscipopt/expr.pxi to explicitly return NotImplemented when the other operand is a numpy.ndarray, and to raise a TypeError for operands that are not genexpr-compatible instead of silently returning NotImplemented. This makes comparisons with NumPy arrays behave predictably and surfaces clear errors for unsupported types.

Add test_NotImplemented to tests/test_expr.py. The new test verifies that mixing strings with Model variables and generator expressions (created as 1/x) raises TypeError for arithmetic (+, *, /), augmented assignments (+=), and comparisons (<=, >=, ==), ensuring unsupported operand combinations are rejected.

Annotate the 'op' parameter as an int in __richcmp__ for both Expr and GenExpr in src/pyscipopt/expr.pxi. This adds Cython-level typing for the rich comparison operator argument when turning expressions into constraints, reducing Python object overhead and ensuring correct C-level behavior.

Add np.number to Union type annotations for buildGenExprObj and _expr_richcmp so numpy scalar types are treated as compatible GenExpr/Expr inputs. Also reformat the _expr_richcmp signature across multiple lines for readability. No behavior changes beyond broader type compatibility with numpy scalar types.

Introduce numpy import and expand test coverage for interactions between Expr and GenExpr: replace 1/x with sqrt(x) and add assertions for Expr+GenExpr, Expr*GenExpr, in-place +=, and Expr + numpy array. Also tidy import ordering from pyscipopt and pyscipopt.scip. These changes validate operator overloading and interoperability with numpy arrays.

Extend tests in tests/test_expr.py to cover interactions between Expr/GenExpr and numpy arrays and matrix variables. Added assertions for Expr + array and GenExpr + array string representations, and for comparison operators (>=, <=, ==) against a matrix variable (m.addMatrixVar(1)), verifying expected ExprCons representations. Removed a now-redundant x + np.array([1]) test.

src/pyscipopt/expr.pxi

Zeroto521 · 2026-04-02T14:21:58Z

Can you please add a couple tests to this one as well @Zeroto521 ?

All test cases are added.

Joao-Dionisio · 2026-04-03T14:23:05Z

tests/test_expr.py

    with pytest.raises(TypeError):
        x == "1"

-    genexpr = 1 /x


why this change?

the string length of sqrt(x) is less than 1 / x.

I don't understand why that matters

The black code style requires a row string no more than 88. This is the 88 string line-length.
So I choose a shorter one.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/pyscipopt/expr.pxi

src/pyscipopt/scip.pxi

src/pyscipopt/expr.pxi

tests/test_expr.py

Joao-Dionisio · 2026-04-03T14:37:35Z

After addressing Copilot's comments, this can be merged.

Modify tests/test_expr.py to better cover comparison directions: change a genexpr >= '1' assertion to '1' >= genexpr to test the reflected comparison, and add a new assertion that genexpr <= '1' raises TypeError. Ensures both operand orders for <=/>= with a genexpr and a string are validated to raise TypeError.

Replace the previous inline PyNumber_Check heuristic with explicit runtime type inspection: obtain the object's Py_TYPE, treat builtin int/float as numbers, explicitly reject numpy arrays, Expr/GenExpr and Python lists/tuples, and fall back to PyNumber_Check otherwise. Also remove the inline qualifier. This tightens number detection to avoid misclassifying arrays or expression objects as numeric values.

In src/pyscipopt/expr.pxi, replace the explicit Cython cast `1.0 / <double>other * self` with `1.0 / other * self`. This removes the redundant C-level cast when dividing by a numeric Python object, simplifying the code and avoiding potential type/casting issues while preserving the intended behavior.

Add an explicit type check in Model to ensure the provided expr is either an Expr or a numeric value before coercing it via Expr() + expr. If the value is neither, raise a TypeError with an informative message. This prevents silent or invalid coercions and improves error diagnostics when callers pass unsupported types.

Replace a bare except in src/pyscipopt/scip.pxi (inside readStatistics) with except (ValueError, TypeError) to avoid unintentionally catching unrelated exceptions (e.g. KeyboardInterrupt/SystemExit) while still handling float conversion failures for statistic values.

Replace Python-level type comparisons with C API checks: use PyLong_Check and PyFloat_Check to detect ints/floats instead of comparing Py_TYPE(o) to int/float. Also switch the numpy cimport to 'cimport numpy as cnp' and use cnp.PyArray_Check. Add the necessary cimports for PyFloat_Check and PyLong_Check and simplify the _is_number implementation for correctness and performance.

Zeroto521 · 2026-04-06T11:45:26Z

tests/test_expr.py

+    a = np.array([1])
+    assert str(x + a) == "[Expr({Term(x): 1.0, Term(): 1.0})]"
+    # test GenExpr + array
+    assert str(genexpr + a) == "[sum(1.0,sqrt(sum(0.0,prod(1.0,x))))]"


This test case requires #1203

Zeroto521 and others added 19 commits January 22, 2026 13:09

Refactor type checks to use NUMBER_TYPES tuple

eff8a9f

Replaces repeated isinstance checks for numeric types with a shared NUMBER_TYPES tuple. This improves maintainability and consistency in type checking within _expr_richcmp and related code.

Update operator overloads to return NotImplemented for invalid types

a531bb3

Modified __mul__ and __add__ methods in Expr and GenExpr classes to return NotImplemented when the operand is not an instance of EXPR_OP_TYPES, improving type safety and operator behavior.

Update type check in _expr_richcmp function

a784346

Replaces the explicit type tuple with EXPR_OP_TYPES in the type check for 'other' in _expr_richcmp, raising TypeError for unsupported types. This improves maintainability and consistency in type validation.

Refactor division operator in Expr class

7833fb4

Simplified the implementation of __truediv__ and __rtruediv__ by directly using the division operator instead of calling __truediv__ explicitly on generated expression objects.

Refactor Expr arithmetic methods to simplify logic

57b2861

float(Expr) can't return True

Merge branch 'master' into dep/_is_number(self)

582283f

Merge branch 'master' into dep/_is_number(self)

532dee4

use a fixed value for constant

f67ec3b

Replaces random matrix generation with a stacked matrix of zeros and ones in test_matrix_dot_performance to provide more controlled test data.

Merge branch 'master' into expr/notimplemented

1515898

Merge branch 'dep/_is_number(self)' into expr/notimplemented

66fcf0a

remove _is_number

f2dae45

Replaces the custom _is_number function with isinstance checks against NUMBER_TYPES for improved clarity and consistency in type checking throughout expression operations.

Refactor type checks and arithmetic in Expr and GenExpr

9083957

Simplifies type checking and arithmetic operations in Expr and GenExpr classes by removing unnecessary float conversions and reordering type checks. Also improves error handling for exponentiation and constraint comparisons.

Merge branch 'master' into expr/notimplemented

6fcc519

Update changelog for NotImplemented return in Expr classes

106e2f3

Documented that Expr and GenExpr now return NotImplemented when they cannot handle other types in calculations.

Update changelog entry for Expr and GenExpr operators

736bc0d

Clarified the changelog note to specify that NotImplemented is returned for Expr and GenExpr operators when they can't handle input types in calculations.

Zeroto521 mentioned this pull request Jan 31, 2026

Feature: Merge Expr and GenExpr #1074

Open

Zeroto521 added 2 commits January 31, 2026 22:44

Fix type checks and error messages in expr and scip modules

59591fc

Corrects error messages in Expr and GenExpr exponentiation to display the correct variable. Removes an unnecessary assertion in Model and replaces a call to _is_number with isinstance for type checking in readStatistics.

Fix type check in _expr_richcmp function

4859daa

Replaces EXPR_OP_TYPES with GENEXPR_OP_TYPES in the type check to ensure correct type validation in the _expr_richcmp method.

Zeroto521 commented Jan 31, 2026

View reviewed changes

src/pyscipopt/expr.pxi Show resolved Hide resolved

Zeroto521 commented Jan 31, 2026

View reviewed changes

src/pyscipopt/expr.pxi Show resolved Hide resolved

Ensure exponent base is float in GenExpr

9c9355f

Casts the exponent base to float in GenExpr's __pow__ method to prevent type errors when reformulating expressions using log and exp.

Zeroto521 changed the title ~~Return NotImplemented for Expr and GenExpr operators~~ Return NotImplemented for Expr and GenExpr operators to simplify Jan 31, 2026

Zeroto521 added 3 commits January 31, 2026 23:03

Ensure float conversion in exponentiation

66b27c5

Explicitly converts 'other' to float in the exponentiation method to avoid type errors when using non-float numeric types.

Remove _is_number from incomplete stubs

3c88e82

The _is_number symbol was removed from the list of incomplete stubs in scip.pyi, likely because it is no longer needed or has been implemented.

Fix multiplication with numeric types in Expr

cad83ff

Ensures that multiplication of Expr by numeric types consistently uses float conversion, preventing potential type errors when multiplying terms.

Copilot AI review requested due to automatic review settings February 1, 2026 02:28

Zeroto521 added 9 commits April 1, 2026 22:21

Improve TypeError message in buildGenExprObj

253c2b4

Convert _expr_richcmp to cython function

1450215

Zeroto521 commented Apr 2, 2026

View reviewed changes

src/pyscipopt/expr.pxi Show resolved Hide resolved

Merge branch 'master' into expr/notimplemented

62e0978

Joao-Dionisio enabled auto-merge (squash) April 3, 2026 14:20

Joao-Dionisio reviewed Apr 3, 2026

View reviewed changes

Joao-Dionisio requested a review from Copilot April 3, 2026 14:24

Copilot started reviewing on behalf of Joao-Dionisio April 3, 2026 14:25 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

src/pyscipopt/expr.pxi Show resolved Hide resolved

src/pyscipopt/scip.pxi Outdated Show resolved Hide resolved

src/pyscipopt/scip.pxi Outdated Show resolved Hide resolved

src/pyscipopt/expr.pxi Outdated Show resolved Hide resolved

tests/test_expr.py Show resolved Hide resolved

Zeroto521 added 3 commits April 5, 2026 09:38

auto-merge was automatically disabled April 5, 2026 02:07
Head branch was pushed to by a user without write access

Zeroto521 added 3 commits April 5, 2026 10:09

Merge branch 'master' into expr/notimplemented

0091af6

Zeroto521 requested a review from Joao-Dionisio April 5, 2026 02:30

Zeroto521 added 2 commits April 5, 2026 14:10

Rename parameter o to x in expr helpers

dbee0f9

Zeroto521 commented Apr 6, 2026

View reviewed changes

Conversation

Zeroto521 commented Jan 31, 2026

Uh oh!

Uh oh!

Uh oh!

Zeroto521 commented Apr 1, 2026

Uh oh!

Uh oh!

Zeroto521 commented Apr 2, 2026

Uh oh!

Joao-Dionisio Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Zeroto521 Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Joao-Dionisio Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Zeroto521 Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Joao-Dionisio commented Apr 3, 2026

Uh oh!

Zeroto521 Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants