Return NotImplemented for Expr and GenExpr operators to simplify

[Zeroto521]

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'

[Zeroto521]

MatrixExpr can handle a vector and a scalar well.

[Zeroto521]

MatrixExpr is a vector container. It could take care of vector and scalar.
Expr and GenExpr should only care about scalar types.

[Copilot]

Pull request overview

This pull request refactors operator overloading in Expr and GenExpr classes to follow Python's standard protocol by returning NotImplemented for unsupported operand types instead of explicitly checking types and raising errors. This allows Python's reflection mechanism to properly handle mixed-type operations.

Changes:

• Removed the _is_number() helper function and replaced it with explicit isinstance() checks against new type tuples (NUMBER_TYPES, EXPR_OP_TYPES, GENEXPR_OP_TYPES)
• Modified operators (__add__, __iadd__, __mul__, __truediv__, __rtruediv__, __rpow__) to return NotImplemented for unsupported types
• Updated _expr_richcmp() to return NotImplemented for numpy arrays
• Simplified buildGenExprObj() by removing numpy array handling and adding explicit type validation
• Updated type annotations to clarify expected input types

Reviewed changes

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

File	Description
src/pyscipopt/scip.pyi	Removed _is_number import from type stub
src/pyscipopt/scip.pxi	Removed assertion using _is_number in setObjective(), replaced _is_number() call with isinstance() check in readStatistics() (but incorrectly)
src/pyscipopt/expr.pxi	Main refactoring: removed _is_number() function, added type tuples, updated all operator methods to return NotImplemented, simplified type handling logic
CHANGELOG.md	Documented the change

Comments suppressed due to low confidence (1)

src/pyscipopt/expr.pxi:245

• The __pow__ method should return NotImplemented for unsupported types before attempting to call float(other). Currently, if an invalid type is passed, it will raise a TypeError from float() instead of allowing Python to try the reflected operation on the other operand. Add a type check at the beginning of the method similar to other operators.

    def __pow__(self, other, modulo):
        if float(other).is_integer() and other >= 0:
            exp = int(other)
        else: # need to transform to GenExpr
            return buildGenExprObj(self)**other

---

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