Opened 5 years ago
Closed 5 years ago
#15414 closed defect (fixed)
Cleanup in numerical.knapsack
Reported by: | ncohen | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.1 |
Component: | numerical | Keywords: | |
Cc: | defeo | Merged in: | |
Authors: | Nathann Cohen | Reviewers: | Luca De Feo |
Report Upstream: | N/A | Work issues: | |
Branch: | u/ncohen/15414 (Commits) | Commit: | 24c57700e67e74a7959a9585cbfe590bb1a6bde1 |
Dependencies: | #15403 | Stopgaps: |
Description
This patch removes a copy of a paragraph that appears twice in numerical.knapsack, adds some backticks from place to place and solver/verbose optional arguments to the knapsack
function.
Change History (12)
comment:1 Changed 5 years ago by
- Branch set to u/ncohen/15414
- Component changed from PLEASE CHANGE to numerical
- Status changed from new to needs_review
comment:2 Changed 5 years ago by
- Commit set to da8b659b96e04b17842b994263694a667234db83
comment:3 Changed 5 years ago by
Hmm... "usefulnesss" is certainly wrong. I'd suggest "usefulnesses", but of course this is not in the dictionary. Any native speaker to the rescue?
comment:4 Changed 5 years ago by
Building the docs, I get
[numerical] /home/dfl/sage/local/lib/python2.7/site-packages/sage/numerical/knapsack.py: docstring of sage.numerical.knapsack:34: WARNING: Bullet list ends without a blank line; unexpected unindent.
But I really don't understand what it is complaining about. The docstring looks good to me around line 34.
comment:5 Changed 5 years ago by
- Reviewers set to Luca De Feo
- Status changed from needs_review to needs_work
sage.numerical.mip.MixedIntegerLinearProgram.solve
says the parameter solver
is deprecated. It'd be better to only leave the link to sage.numerical.mip.MixedIntegerLinearProgram
, where the solver
parameter to the class is documented.
comment:6 Changed 5 years ago by
Ok, last one. Shouldn't the default value for verbose
be None
? This looks more coherent with the doc of solve
.
comment:7 Changed 5 years ago by
Allllllllll right ! I fixed them all by updating my commit. Except for your comment about "verbose". This variable has levels which can be integers, so 0 makes sense :-)
Nathann
comment:8 Changed 5 years ago by
- Commit changed from da8b659b96e04b17842b994263694a667234db83 to 24c57700e67e74a7959a9585cbfe590bb1a6bde1
comment:9 Changed 5 years ago by
- Milestone changed from sage-5.13 to sage-6.0
- Status changed from needs_work to positive_review
You rewrote history! I saw it, you did it! (
not that I really care :)
I don't know how you fixed the warning, but you sure did! I have no more objections, let it go!
comment:10 Changed 5 years ago by
Yooooooo !!
There was ONE space missing at the beginning of the line
to 0 by default, which means quiet.
And now that history is rewritten, you may know it but you will never be able to prove it. And soon you will forget....
AHAHAHAHAHHAHAHAHAAAAAAAAAAAAAAAAHAHAHAHAHAHAAAAAAAAAAAAAA !!!
Nathann
comment:11 Changed 5 years ago by
- Milestone changed from sage-6.0 to sage-6.1
comment:12 Changed 5 years ago by
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits: