Opened 8 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)

trac_12340.patch​ (8.7 KB) - added by benjaminfjones 7 years ago.
mark doctests requiring polymake correctly

Download all attachments as: .zip

Change History (7)

comment:1 Changed 8 years ago by was

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.

comment:2 Changed 7 years ago by benjaminfjones

I'm working on this, looks like polymake appears in geometry/polytope.py, rings/polynomial/groebner_fan.py, and rings/polynomial/multi_polynomial.pyx.

Changed 7 years ago by benjaminfjones

mark doctests requiring polymake correctly

comment:3 Changed 7 years ago by benjaminfjones

  • 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 mhansen

  • Authors set to Benjamin Jones
  • Reviewers set to Mike Hansen
  • Status changed from needs_review to positive_review

Looks good to me.

comment:5 Changed 7 years ago by mhansen

  • Keywords sd40.5 added

comment:6 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.1.beta3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.