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: | sage-7.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 non-variables.
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 follow-up: ↓ 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 accepts 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 non-variables.New commits:
MixedIntegerLinearProgram.get_values: Input checking