Skip to content

Conversation

Zeroto521
Copy link
Contributor

closes #1021

Copy link
Contributor Author

@Zeroto521 Zeroto521 left a comment

Choose a reason for hiding this comment

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

The code style and document style should check.

@Joao-Dionisio Joao-Dionisio self-requested a review July 13, 2025 18:50
@Joao-Dionisio
Copy link
Member

Thank you, this looks pretty good! Just some minor comments, really

Condensed the addMatrixConsIndicator method signature to a single line and updated the docstring for improved clarity and consistency. Parameter descriptions were streamlined and default values were explicitly stated.
Copy link

codecov bot commented Jul 14, 2025

Codecov Report

Attention: Patch coverage is 69.04762% with 26 lines in your changes missing coverage. Please review.

Project coverage is 54.33%. Comparing base (6452ea6) to head (21e4097).
Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
src/pyscipopt/scip.pxi 69.04% 26 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1024      +/-   ##
==========================================
+ Coverage   54.13%   54.33%   +0.20%     
==========================================
  Files          22       22              
  Lines        5045     5192     +147     
==========================================
+ Hits         2731     2821      +90     
- Misses       2314     2371      +57     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Specified the return type of the Model.add_matrix_constraint method as MatrixConstraint to improve type clarity and support better static analysis.
Copy link
Member

@Joao-Dionisio Joao-Dionisio left a comment

Choose a reason for hiding this comment

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

Yeahh I like the code duplication even worse now, but the alternative I'm thinking of would introduce some inconsistency. We'll leave it as is

@Joao-Dionisio Joao-Dionisio merged commit ed56984 into scipopt:master Jul 15, 2025
1 check passed
@Zeroto521 Zeroto521 deleted the model/addMatrixConsIndicator branch July 16, 2025 07:18
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.

Feture: addMatrixConsIndicator, Indicator constraint for Matrix

2 participants