Skip to content

Conversation

@Joao-Dionisio
Copy link
Member

There were a bunch of issues and PRs related to addConsNode() not behaving as expected (#391, #580, #605), and the current implementation forces users to use an event handler whenever they want to implement a slightly more complicated branching decision. In this event handler, they then use addConsLocal()

This is a breaking change, but I feel like it should be done.
A middle ground would be to accept both ExprCons and Constraint, but it doesn't feel very elegant. addCons() doesn't do this. Any thoughts, @mmghannam ?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR changes addConsNode() and addConsLocal() to accept ExprCons instead of Constraint, aligning their API with addCons() and addressing issues from previous PRs (#391, #580, #605). The functions now also return the created Constraint object for user reference.

Changes:

  • Modified addConsNode() and addConsLocal() to accept ExprCons expressions instead of Constraint objects
  • Added return values to both functions, returning the created Constraint object
  • Added comprehensive test coverage in a new test file test_addconsnode.py

Reviewed changes

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

File Description
src/pyscipopt/scip.pxi Refactored addConsNode() and addConsLocal() to accept ExprCons, follow the addCons pattern for constraint creation, and return Constraint objects
tests/test_addconsnode.py Added new comprehensive tests for both addConsNode and addConsLocal with custom branching rules
CHANGELOG.md Documented the breaking change in the Changed section

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants