Opened 6 years ago
Closed 6 years ago
#20304 closed defect (fixed)
More error checking in MixedIntegerLinearProgram
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage7.2 
Component:  numerical  Keywords:  lp 
Cc:  dimpase, ncohen  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  7d7aeed (Commits, GitHub, GitLab)  Commit:  7d7aeed0d1ccfbb1651f8b296f9913b0db5d1b6b 
Dependencies:  Stopgaps: 
Description (last modified by )
The get_values
method silently ignored nonvariables.
Change History (16)
comment:1 Changed 6 years ago by
 Branch set to u/mkoeppe/more_error_checking_in_mixedintegerlinearprogram
comment:2 Changed 6 years ago by
 Cc dimpase ncohen added
 Commit set to dc3895bb02227dff5b2f7130e43f877754c4861b
 Component changed from PLEASE CHANGE to numerical
 Keywords lp added
 Status changed from new to needs_review
 Type changed from PLEASE CHANGE to defect
comment:3 Changed 6 years ago by
 Description modified (diff)
comment:4 Changed 6 years ago by
 Reviewers set to Vincent Delecroix
 Status changed from needs_review to needs_work
Please use new style error ValueError(msg)
instead of ValueError, msg
(Python 3 compatibility).
Would be better to use repr rather than str. The output of "{!r}".format(l)
is nicer than str(l)
. As you can see
sage: l = 1 sage: str(l) '1' sage: "{!r}".format(l) '1' sage: l = '1' sage: str(l) '1' sage: "{!r}".format(l) "'1'"
The following are not caught correctly
sage: M1 = MixedIntegerLinearProgram() sage: M2 = MixedIntegerLinearProgram() sage: x = M1.new_variable() sage: y = M1.new_variable() sage: z = M2.new_variable() sage: M2.add_constraint(z[0] <= 5) sage: M2.solve() 0.0 sage: M2.get_values(x) # should be a ValueError Traceback (most recent call last): ... KeyError: x_0 sage: M2.get_values(y) # should be a ValueError {}
comment:5 Changed 6 years ago by
And if the input is not a variable it would be better to raise a TypeError
rather than a ValueError
.
comment:6 Changed 6 years ago by
 Commit changed from dc3895bb02227dff5b2f7130e43f877754c4861b to 4290bde5ae4b40b8ae434fce93db624c4c76d873
Branch pushed to git repo; I updated commit sha1. New commits:
4290bde  MixedIntegerLinearProgram: use new style error ValueError(msg) instead of ValueError, msg (Python 3 compatibility).

comment:7 Changed 6 years ago by
 Commit changed from 4290bde5ae4b40b8ae434fce93db624c4c76d873 to 13e82d9944fbf5f64c6ea6559a7c6be221aaea09
comment:8 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:9 Changed 6 years ago by
Nice.
Last thing: the first error should be a ValueError
. The type is indeed the good one!
comment:10 followup: ↓ 11 Changed 6 years ago by
Should I then make it a ValueError
also when it's a string, but not one that names a variable?
(If so, what's the right way to test if it is a string?)
comment:11 in reply to: ↑ 10 Changed 6 years ago by
Replying to mkoeppe:
Should I then make it a
ValueError
also when it's a string, but not one that names a variable? (If so, what's the right way to test if it is a string?)
Not necessarily (one can assume that the "good" way of calling this function is with a MIP variable). Though, to test if the object x
is a string you can use isinstance(x, str)
(which returns a boolean).
comment:12 Changed 6 years ago by
I was confused, it actually does not accept strings  only MIPVariable objects.
comment:13 Changed 6 years ago by
 Commit changed from 13e82d9944fbf5f64c6ea6559a7c6be221aaea09 to 7d7aeed0d1ccfbb1651f8b296f9913b0db5d1b6b
Branch pushed to git repo; I updated commit sha1. New commits:
7d7aeed  MixedIntegerLinearProgram.get_values: Raise TypeError when not a MIPVariable, ValueError when from the wrong MIP

comment:14 Changed 6 years ago by
 Status changed from needs_review to positive_review
comment:15 Changed 6 years ago by
Thanks for reviewing!
comment:16 Changed 6 years ago by
 Branch changed from u/mkoeppe/more_error_checking_in_mixedintegerlinearprogram to 7d7aeed0d1ccfbb1651f8b296f9913b0db5d1b6b
 Resolution set to fixed
 Status changed from positive_review to closed
The
get_values
method silently ignored nonvariables.New commits:
MixedIntegerLinearProgram.get_values: Input checking