Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

abs as global function #357

Merged
merged 3 commits into from
Jun 26, 2023
Merged

abs as global function #357

merged 3 commits into from
Jun 26, 2023

Conversation

Wout4
Copy link
Collaborator

@Wout4 Wout4 commented Jun 22, 2023 •
edited
Loading

abs as a global, might have forgotten some cleanups?

@Wout4 Wout4 requested review from tias and JoD and removed request for tias June 22, 2023 23:19
Dimosts
Dimosts reviewed Jun 22, 2023
View reviewed changes
Copy link
Collaborator

@Dimosts Dimosts left a comment

Choose a reason for hiding this comment

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

I think it is almost ok, easy to review, could be merged before freeze. Just a couple of comments for consistency.

tests/test_globalconstraints.py Outdated Show resolved Hide resolved
tests/test_globalconstraints.py Show resolved Hide resolved
@Wout4
use cp.abs
ab9142c 
added tests
added abs to init
@Wout4 Wout4 added the next release Needed for next release label Jun 26, 2023
@JoD
Copy link
Collaborator

JoD commented Jun 26, 2023 •
edited
Loading

Ran test_constraints.py with the exact branch merged with this one. No issues, so good for me.

The only concern I have is that we might break bigtest even more? Can we fix this abs refactor as easily as the GlobalFunctions refactor for bigtest?

@Wout4
Copy link
Collaborator Author

Wout4 commented Jun 26, 2023

aha that is indeed annoying.. I think the only way out would be to allow abs as an operator as well but that's a bit ugly

@Wout4 Wout4 linked an issue Jun 26, 2023 that may be closed by this pull request
abs should become a global constraint #258
Closed
@Wout4 Wout4 merged commit 43cc57d into master Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dimosts Dimosts Dimosts left review comments

@JoD JoD Awaiting requested review from JoD

Assignees
No one assigned
Labels
next release Needed for next release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

abs should become a global constraint
3 participants
@Wout4 @JoD @Dimosts