python_builtins: follow python signature #503
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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).
|
something to maybe add to this... we don't overwrite 'abs', but using abs(decis_var) just works. However, if you do So maybe we should fake-override abs too, e.g. just |
|
looks good, I think this pr adresses the axis arguments: #492 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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).