#7311: Improve the add_constraint method from MixedIntegerLinearProgram
-------------------------+--------------------------------------------------
Reporter: ncohen | Owner: jkantor
Type: defect | Status: needs_review
Priority: critical | Milestone: sage-4.3.4
Component: numerical | Keywords:
Author: | Upstream: N/A
Reviewer: | Merged:
Work_issues: |
-------------------------+--------------------------------------------------
Comment(by abmasse):
I looked once again at your patch. It seems correct, but I have a few
comments:
1. Around line 711, you write
{{{
f = linear_function.f
}}}
Woudn't be better to access this value by a function that LinearFunction
provides ? More precisely, LinearFunction objects should have a method
`f()` or `function()` and a private attribute `_f`. This is not a big
problem, but seems cleaner. Same remark with `linear_function.constraints`
of line 733 and `linear_function.equality` of line 735.
2. This is more a style remark. You tend to put a lot of space in your
code, but I feel there is too much, which causes the body of the functions
to span more lines than terminals can display.
Short of that, the code seems ok, and all tests even optional pass. There
are some typos in the doc, but I can fix it while reviewing. As soon as
you answer/correct item 1, I should be able to finish reviewing it.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/7311#comment:13>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
