From 5bd8a6e3347c6f54320fec899adbaa706fad5a7b Mon Sep 17 00:00:00 2001
From: Tias Guns <tias@ulyssis.org>
Date: Tue, 16 Jul 2024 10:14:59 +0200
Subject: [PATCH] python_builtins: follow python signature

our 'min' and 'max' didn't allow multiple arguments, while Python's
does

in general we were hesitating whether our min/max/sum should mimic
Python's or NumPys. Now it was a mix in the middle... we were somewhat
like Python but did np.max() inside (same for sum, messing up the return
type); and we didnt yet get to implementing 'axis' like numpy either.

With this pull request, I propose that the least confusing thing for
people that do `from cpmpy import *` is to not break any existing
python code... that is, follow the Python signature of min/max/sum
(and fall back to that built-in when no decision variables are given).
---
 cpmpy/expressions/python_builtins.py | 48 ++++++++++++++++++----------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/cpmpy/expressions/python_builtins.py b/cpmpy/expressions/python_builtins.py
index caa26bb71..87fb6a47e 100644
--- a/cpmpy/expressions/python_builtins.py
+++ b/cpmpy/expressions/python_builtins.py
@@ -18,7 +18,6 @@
         min
         sum
 """
-import numpy as np
 import builtins  # to use the original Python-builtins
 
 from .utils import is_false_cst, is_true_cst
@@ -70,7 +69,7 @@ def any(iterable):
         elif isinstance(elem, Expression) and elem.is_bool():
             collect.append(elem)
         else:
-            raise Exception("Non-Boolean argument '{}' to 'all'".format(elem))
+            raise Exception("Non-Boolean argument '{}' to 'any'".format(elem))
     if len(collect) == 1:
         return collect[0]
     if len(collect) >= 2:
@@ -78,33 +77,50 @@ def any(iterable):
     return False
 
 
-def max(iterable):
+def max(*iterable, **kwargs):
     """
-        max() overwrites python built-in,
-        checks if all constants and computes np.max() in that case
+        max() overwrites the python built-in to support decision variables.
+
+        if iterable does not contain CPMpy expressions, the built-in is called
+        else a Maximum functional global constraint is constructed; no keyword
+          arguments are supported in that case
     """
+    if len(iterable) == 1:
+        iterable = tuple(iterable[0])
     if not builtins.any(isinstance(elem, Expression) for elem in iterable):
-        return np.max(iterable)
+        return builtins.max(*iterable, **kwargs)
+
+    assert len(kwargs)==0, "max over decision variables does not support keyword arguments"
     return Maximum(iterable)
 
 
-def min(iterable):
+def min(*iterable, **kwargs):
     """
-        min() overwrites python built-in,
-        checks if all constants and computes np.min() in that case
+        min() overwrites the python built-in to support decision variables.
+
+        if iterable does not contain CPMpy expressions, the built-in is called
+        else a Minimum functional global constraint is constructed; no keyword
+          arguments are supported in that case
     """
+    if len(iterable) == 1:
+        iterable = tuple(iterable[0])
     if not builtins.any(isinstance(elem, Expression) for elem in iterable):
-        return np.min(iterable)
+        return builtins.min(*iterable, **kwargs)
+
+    assert len(kwargs)==0, "min over decision variables does not support keyword arguments"
     return Minimum(iterable)
 
 
-def sum(iterable):
+def sum(iterable, **kwargs):
     """
-        sum() overwrites python built-in,
-        checks if all constants and computes np.sum() in that case
-        otherwise, makes a sum Operator directly on `iterable`
+        sum() overwrites the python built-in to support decision variables.
+
+        if iterable does not contain CPMpy expressions, the built-in is called
+        checks if all constants and uses built-in sum() in that case
     """
-    iterable = list(iterable) # Fix generator polling
+    iterable = tuple(iterable) # Fix generator polling
     if not builtins.any(isinstance(elem, Expression) for elem in iterable):
-        return np.sum(iterable)
+        return builtins.sum(iterable, **kwargs)
+
+    assert len(kwargs)==0, "sum over decision variables does not support keyword arguments"
     return Operator("sum", iterable)