Opened 7 years ago
Closed 7 years ago
#12340 closed defect (fixed)
the optional polymake doctests are seriously misformatted so never run
Reported by: | was | Owned by: | mvngu |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.1 |
Component: | doctest coverage | Keywords: | sd40.5 |
Cc: | Merged in: | sage-5.1.beta3 | |
Authors: | Benjamin Jones | Reviewers: | Mike Hansen |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
sage: search_src('polymake')
reveals many tests marked # optional: needs polymake
.
These will *NOT* get run by doing
sage -t -only_optional=polymake geometry/polytope.py
They should be formatted as
#optional - polymake
By the way, this test works fine without polymake is installed, because it doesn't do anything, so improve that by adding another test right after this that actually exercise the optional package.
sage: P = polymake.convex_hull([[1,0,0,0], [1,0,0,1], [1,0,1,0], [1,0,1,1], [1,1,0,0], [1,1,0,1], [1,1,1,0], [1,1,1,1]])
Attachments (1)
Change History (7)
comment:1 Changed 7 years ago by
comment:2 Changed 7 years ago by
I'm working on this, looks like polymake appears in geometry/polytope.py
, rings/polynomial/groebner_fan.py
, and rings/polynomial/multi_polynomial.pyx
.
comment:3 Changed 7 years ago by
- Status changed from new to needs_review
I could add doctests to a bunch of functions here, but I can't get the the polymake spkg to build on pretty much any platform, see #5488. But in the meantime, the attached patch corrects the optional doctests.
comment:4 Changed 7 years ago by
- Reviewers set to Mike Hansen
- Status changed from needs_review to positive_review
Looks good to me.
comment:5 Changed 7 years ago by
- Keywords sd40.5 added
comment:6 Changed 7 years ago by
- Merged in set to sage-5.1.beta3
- Resolution set to fixed
- Status changed from positive_review to closed
By the way, this code is from my commit 271, i.e., ancient history, before there was an infrastructure for how optional package tests worked. But it would be good to update this.