#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):
Hello again Nathann !
I looked once again at your patch. You'll see that I added one on top of
yours: it only modifies the formatting in the code and in the
documentation, nothing major. In particular, I removed empty lines where
they didn't seem necessary to me. I should give a positive review to your
ticket very soon, but before that, there are some questions I would like
you to answer.
1. There is a parameter `name` to the `add_constraint` function of
`MixedIntegerLinearProgram`. Why do you need such a parameter ? I'm sure
there is a good reason, but it would be nice to illustrate by an example
why this is helpful.
2. Around line 715, you have {{{constant_coefficient = f.pop(-1,0)}}}.
Isn't it dangerous ? If I'm not mistaken, this modifies the parameter
`linear_function` and could have unsuspected side effects ?
3. Since functions such as `__init__`, `__eq__`, etc. do not appear in the
documentation, it would be better to have more detailed documentation just
under the declaration of the class. Instead of {{{A class to represent
Linear Constraints}}}, you could describe in detail what it is used for
and give some examples.
Sorry again for bothering you ! Don't forget to apply my patch if you want
to add some modifications to yours. Next time should be a positive review
!
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/7311#comment:15>
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.
