#22452 closed enhancement (fixed)
Create a Polymake pexpect interface
Reported by:  Simon King  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  packages: optional  Keywords:  Polymake, days84 
Cc:  Vincent Delecroix, JeanPhilippe Labbé  Merged in:  
Authors:  Simon King  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  52a77ee (Commits, GitHub, GitLab)  Commit:  
Dependencies:  #22501  Stopgaps: 
Description
There is PyPolymake, to be added at #21170. But I think it wouldn't hurt to additionally have a pexpect interface to Polymake. And the purpose of this ticket is to create one.
Attachments (3)
Change History (105)
comment:1 Changed 6 years ago by
Keywords:  days84 added; days85 removed 

comment:2 Changed 6 years ago by
Branch:  → u/SimonKing/create_a_polymake_pexpect_interface 

comment:3 Changed 6 years ago by
Cc:  Vincent Delecroix added 

Commit:  → c3add962d2d518164765993b70e843d10f17a4da 
comment:4 Changed 6 years ago by
Here I am showcasing some things that work already:
sage: from sage.interfaces.polymake import polymake sage: polymake.help("objects/Cone/properties/Combinatorics/RAYS_IN_FACETS") property RAYS_IN_FACETS : IncidenceMatrix<NonSymmetric> Rayfacet incidence matrix, with rows corresponding to facets and columns to rays. Rays and facets are numbered from 0 to N_RAYS1 rsp. N_FACETS1, according to their order in RAYS rsp. FACETS. sage: polymake.help('rand_sphere') functions/Producing a polytope from scratch/rand_sphere: rand_sphere(d, n; Options) > Polytope Produce a ddimensional polytope with n random vertices uniformly distributed on the unit sphere. Arguments: Int d the dimension Int n the number of random vertices Options: seed => Int controls the outcome of the random number generator; fixing a seed number guarantees the same outcome. Returns Polytope
So, there is access to Polymake' help system.
sage: P = polymake.rand<tab key> polymake.rand01 polymake.rand_knot polymake.rand_aof polymake.rand_metric polymake.rand_box polymake.rand_metric_int > polymake.rand_cyclic polymake.rand_perm polymake.rand_inner_points polymake.rand_seed
So, tab completion of the interface itself works, it yields ALL function names, including those that are not part of the current Polymake application.
sage: P = polymake.rand_sphere(3,10,seed=5)
So, calling a function with parameters works
sage: P Random spherical polytope of dimension 3; seed=5
So, string representation works, actually nicer than in Polymake (more examples below).
sage: P.known_properties() # members computed so far ['BOUNDED', 'CONE_AMBIENT_DIM', 'FEASIBLE', 'POINTS'] sage: P.get_schedule('F_VECTOR') precondition : BOUNDED ( lrs.convex_hull.count: N_FACETS : RAYS  INPUT_RAYS ) lrs.convex_hull.count: N_FACETS : RAYS  INPUT_RAYS sensitivity check for VertexPerm cdd.convex_hull.canon: POINTED, RAYS, LINEALITY_SPACE : INPUT_RAYS CONE_DIM : RAYS  INPUT_RAYS N_RAYS : RAYS precondition : POINTED ( LINEALITY_DIM, LINEALITY_SPACE : ) LINEALITY_DIM, LINEALITY_SPACE : COMBINATORIAL_DIM : CONE_DIM, LINEALITY_DIM precondition : COMBINATORIAL_DIM ( F_VECTOR : N_FACETS, N_RAYS, COMBINATORIAL_DIM ) F_VECTOR : N_FACETS, N_RAYS, COMBINATORIAL_DIM
So, this shows how the fvector would be computed for this polytope.
sage: V = P.VER<tab key> P.VERTEX_BARYCENTER P.VERTICES P.VERTEX_LABELS P.VERTICES_IN_FACETS P.VERTEX_NORMALS P.VERTICES_IN_INEQUALITIES P.VERTEX_SIZES P.VERY_AMPLE
So, tab completion for Polymake elements works. It shows both members (including those that aren't computed yet) and functions (including those that won't be applicable to the object).
sage: set_verbose(3) sage: V = P.VERTICES sage: F = P.F_VECTOR polymake: used package lrs Implementation of the reverse search algorithm of Avis and Fukuda. Copyright by David Avis. http://cgm.cs.mcgill.ca/~avis/lrs.html
So, when verbosity is set to a positive value, comments printed by Polymake will be printed by Sage.
sage: F 10 24 16 sage: V 1 549467720344047/2251799813685248 1666038093248221/2251799813685248 5646952731601237/9007199254740992 1 396443771972991/562949953421312 3761317241461329/36028797018963968 6325385137097525/9007199254740992 1 8731416515056451/9007199254740992 8225904699783425/36028797018963968 6513541817898829/72057594037927936 1 6649740931032245/9007199254740992 316039202166671/562949953421312 6735311418008155/18014398509481984 1 1027872253618593/1125899906842624 4941443956760209/18014398509481984 5443456128447233/18014398509481984 1 3237778007687205/4503599627370496 3127563668227821/4503599627370496 8478171037802839/288230376151711744 1 4654944693569921/9007199254740992 2595293938705027/4503599627370496 5702536786492105/9007199254740992 1 4153090305917205/4503599627370496 2814681273371419/18014398509481984 3186909849764487/9007199254740992 1 6185041161130579/9007199254740992 2775723053730255/4503599627370496 6944451127537363/18014398509481984 1 6106052881221159/18014398509481984 2063008243044317/18014398509481984 8410984913482021/9007199254740992
So, string representation is (at least for the things that are currently wrapped) meaningful.
sage: F[2] 16 sage: V[3][3] 6735311418008155/18014398509481984
So, attribute access works.
sage: V.foobar() Traceback (most recent call last) ... TypeError: polymake: ERROR: Undefined subroutine &Polymake::User::foobar called at input line 1. sage: polymake.eval('foobar(4)') ^CInterrupting Polymake... Traceback (most recent call last) ... KeyboardInterrupt: Ctrlc pressed while running Polymake sage: polymake.eval('foobar(4);') Traceback (most recent call last) ... PolymakeError: polymake: ERROR: Undefined subroutine &Polymake::User::foobar called at input line 1.
So, error handling is implemented.
Meanwhile, more members are known:
sage: P.known_properties() ['AFFINE_HULL', 'BOUNDED', 'COMBINATORIAL_DIM', 'CONE_AMBIENT_DIM', 'CONE_DIM', 'FAR_FACE', 'FEASIBLE', 'FULL_DIM', 'F_VECTOR', 'LINEALITY_DIM', 'LINEALITY_SPACE', 'N_FACETS', 'N_POINTS', 'N_VERTICES', 'POINTED', 'POINTS', 'VERTICES']
Of course, some things won't work yet. For example, I am not catching warnings yet.
comment:5 Changed 6 years ago by
Commit:  c3add962d2d518164765993b70e843d10f17a4da → bfae4863bf12ec555ce7976fc39ce42a16a12fcc 

Branch pushed to git repo; I updated commit sha1. New commits:
bfae486  Implement getting length of arrays and hashes

comment:6 Changed 6 years ago by
Example for the next commit:
sage: from sage.interfaces.polymake import polymake sage: P = polymake.rand_sphere(3,10,seed=5) sage: len(P) # A polytope is in fact internally represented by an array 14 sage: V = P.VERTICES sage: len(V) 10 sage: len(V[2]) 4
New commits:
bfae486  Implement getting length of arrays and hashes

comment:7 Changed 6 years ago by
Cc:  JeanPhilippe Labbé added 

comment:8 Changed 6 years ago by
Now I am totally puzzled. After doing some changes, many thinks didn't work. Thus, I reverted all changes. I even deleted my local git branch and downloaded the branch from here. But things still don't work (tab completion etc). Of course I did sage br.
Do you have any idea what could cause such persisting problems?
comment:9 followup: 12 Changed 6 years ago by
It seems that my Sage installation is totally broken. I did "make start" and it seems that it installs python3 without my asking for it.
comment:10 Changed 6 years ago by
Commit:  bfae4863bf12ec555ce7976fc39ce42a16a12fcc → c3e7d475d39678c941160bedf9033db6fd7e9c85 

Branch pushed to git repo; I updated commit sha1. New commits:
c3e7d47  Fix reading from file. Fix KeyboardInterrupt for Polymake

comment:11 Changed 6 years ago by
Commit:  c3e7d475d39678c941160bedf9033db6fd7e9c85 → 2bd2ac5d3b5931acdfdec516e16a875356b7c646 

Branch pushed to git repo; I updated commit sha1. New commits:
2bd2ac5  add synchronisation code for Polymake interface

comment:12 followup: 13 Changed 6 years ago by
Replying to SimonKing:
It seems that my Sage installation is totally broken. I did "make start" and it seems that it installs python3 without my asking for it.
Yes. That is normal. python2 and python3 do install by default now (since 2 or 3 beta already).
comment:13 Changed 6 years ago by
Replying to vdelecroix:
Replying to SimonKing:
It seems that my Sage installation is totally broken. I did "make start" and it seems that it installs python3 without my asking for it.
Yes. That is normal. python2 and python3 do install by default now (since 2 or 3 beta already).
Thanks, I didn't notice that before.
I still don't know what went wrong, but it is now again working, including the synchronisation code that I posted in the previous commit.
comment:14 Changed 6 years ago by
Commit:  2bd2ac5d3b5931acdfdec516e16a875356b7c646 → 06e85fd9d84b15f06c4aa2e14d4db84c009c0459 

Branch pushed to git repo; I updated commit sha1. New commits:
06e85fd  Keep polymake interface in sync. Take docstring from polymake. Fix calling of polymake functions

comment:15 Changed 6 years ago by
Finally I found a reasonable source explaining a part of perl that seems particularly relevant: http://perldoc.perl.org/perlref.html
So, I am now rewriting a couple of things in terms of what I learn about perl references.
comment:16 Changed 6 years ago by
It is of course conceivable that I am just lacking experience, but I find Perl sucks.
In order to write an interface, it is important to do things uniformly. Hence, all variables created by Sage in Polymake should be "the same". In the current branch, "the same" means that they are all references (which in Perl means they are SCALAR).
So, how to assign any object x in Polymake to a variable var?
If you define a SCALAR "var" and assign to "x", you of course do not get a reference to x but some scalar interpretation of whatever is in x. Thus, I need to create a reference to x. Sounds trivial (and perhaps it is). However, all what I found is that one needs to do $var=\@x
, $var=\%x
or $var=\*x
depending on whether x is an array, a hash or an io stream. OK, sounds easy. But so far I found no way to make Perl tell me what x really is.
Instead, let var uniformly be an array whose single entry is x. Thus:
polytope > @P = [rand_sphere(4,33, seed=>5)]; polytope > print @P[0]; polymake: ERROR: Scalar value @Polymake::User::P[0] better written as $Polymake::User::P[0] at input line 1.
WTF?? I should better write something that I did not write by the same thing??? From what I thought I have learnt about Perl syntax, @P[0]
should give the first entry in the array P!
comment:17 Changed 6 years ago by
Aha, my mistake. Apparently it works slightly differently:
polytope > @P = (rand_sphere(4,33, seed=>5)); polytope > print $P[0]; Polymake::polytope::Polytope__Rational=ARRAY(0x80d0370) polytope > print $P[0][1]; Random spherical polytope of dimension 4; seed=5
So, creating a new variable that points to the content of the old variable should work like this, and it does:
polytope > @Q = ($P[0]); polytope > print $Q[0]; Polymake::polytope::Polytope__Rational=ARRAY(0x80d0370) polytope > print $Q[0][1]; Random spherical polytope of dimension 4; seed=5
Access to members also works as expected:
polytope > @V = ($Q[0]>VERTICES); ... polytope > print $V[0]; ... 1 2185542006599419/2251799813685248 5220912692532935/36028797018963968 2637694986535851/72057594037927936 3401062251409073/18014398509481984 polytope > @S = ($Q[0]>get_schedule("H_VECTOR")); polymake: used package ppl The Parma Polyhedra Library (PPL): A C++ library for convex polyhedra and other numerical abstractions. http://www.cs.unipr.it/ppl/ polytope > print join(", ", $S[0]>list); precondition : N_RAYS  N_INPUT_RAYS ( ppl.convex_hull.primal: FACETS, LINEAR_SPAN : RAYS  INPUT_RAYS ), sensitivity check for FacetPerm, ppl.convex_hull.primal: FACETS, LINEAR_SPAN : RAYS  INPUT_RAYS, RAYS_IN_FACETS : RAYS, FACETS, GRAPH.ADJACENCY : RAYS_IN_FACETS, DUAL_GRAPH.ADJACENCY : RAYS_IN_FACETS, SIMPLICIAL : COMBINATORIAL_DIM, RAYS_IN_FACETS, N_EDGES : ADJACENCY ( applied to DUAL_GRAPH ), N_EDGES : ADJACENCY ( applied to GRAPH ), N_RAYS : RAYS, N_FACETS : FACETS, precondition : COMBINATORIAL_DIM ( F_VECTOR : N_FACETS, N_RAYS, GRAPH.N_EDGES, DUAL_GRAPH.N_EDGES, COMBINATORIAL_DIM ), F_VECTOR : N_FACETS, N_RAYS, GRAPH.N_EDGES, DUAL_GRAPH.N_EDGES, COMBINATORIAL_DIM, precondition : SIMPLICIAL ( H_VECTOR : F_VECTOR ), H_VECTOR : F_VECTOR
So, I'll rewrite my branch accordingly.
comment:18 Changed 6 years ago by
Argh. The same trick won't work when I want a list in a list:
polytope > print join(", ", $L[0]); precondition : N_RAYS  N_INPUT_RAYS ( ppl.convex_hull.primal: FACETS, LINEAR_SPAN : RAYS  INPUT_RAYS )
comment:19 Changed 6 years ago by
Next thing to try: Use a reference to an array. According to http://perldoc.perl.org/5.20.0/perllol.html, I expect it to work as follows:
polytope > $P = [rand_sphere(4,33, seed=>5)]; polytope > print $P>[0]; Polymake::polytope::Polytope__Rational=ARRAY(0x7c9bae0) polytope > print $P>[0][1]; Random spherical polytope of dimension 4; seed=5 polytope > $Q = [$P>[0]]; polytope > print $Q>[0]; Polymake::polytope::Polytope__Rational=ARRAY(0x7c9bae0) polytope > print $Q>[0][1]; Random spherical polytope of dimension 4; seed=5 polytope > $V = [$Q>[0]>VERTICES]; polytope > print $V>[0]; ... polytope > $S = [$Q>[0]>get_schedule("H_VECTOR")]; polytope > print join(", ", $S>[0]>list); precondition : N_RAYS  N_INPUT_RAYS ( ppl.convex_hull.primal: FACETS, LINEAR_SPAN : RAYS  INPUT_RAYS ), sensitivity check for FacetPerm, ppl.convex_hull.primal: FACETS, LINEAR_SPAN : RAYS  INPUT_RAYS, RAYS_IN_FACETS : RAYS, FACETS, GRAPH.ADJACENCY : RAYS_IN_FACETS, DUAL_GRAPH.ADJACENCY : RAYS_IN_FACETS, SIMPLICIAL : COMBINATORIAL_DIM, RAYS_IN_FACETS, N_EDGES : ADJACENCY ( applied to DUAL_GRAPH ), N_EDGES : ADJACENCY ( applied to GRAPH ), N_RAYS : RAYS, N_FACETS : FACETS, precondition : COMBINATORIAL_DIM ( F_VECTOR : N_FACETS, N_RAYS, GRAPH.N_EDGES, DUAL_GRAPH.N_EDGES, COMBINATORIAL_DIM ), F_VECTOR : N_FACETS, N_RAYS, GRAPH.N_EDGES, DUAL_GRAPH.N_EDGES, COMBINATORIAL_DIM, precondition : SIMPLICIAL ( H_VECTOR : F_VECTOR ), H_VECTOR : F_VECTOR polytope > $L = [$S>[0]>list]; polytope > print join(", ", $L>[0]); precondition : N_RAYS  N_INPUT_RAYS ( ppl.convex_hull.primal: FACETS, LINEAR_SPAN : RAYS  INPUT_RAYS )
So, for wrapping lists, it won't work either.
comment:20 followup: 21 Changed 6 years ago by
It seems that the problem with nested arrays is known and is discussed here.
comment:21 Changed 6 years ago by
Apparently there are no arrays of arrays in Perl. Instead, each item in an array has to be a scalar. Hence, an array of arrays would in fact be implemented as an array of references to arrays.
And it seems that Perl would automagically create such references if you assign something to an entry of an existing array, according to the reference given in the previous post. So, I expect that things would finally work as follows:
polytope > $P = []; $P>[0] = rand_sphere(4,33,seed=>5); polytope > print $P>[0][1]; Random spherical polytope of dimension 4; seed=5 polytope > $Q = []; $Q>[0] = $P>[0]; polytope > print $Q>[0][1]; Random spherical polytope of dimension 4; seed=5 polytope > $V = []; $V>[0] = $Q>[0]>VERTICES; polytope > print $V>[0]; ... # output as expected polytope > $S = []; $S>[0] = $Q>[0]>get_schedule("H_VECTOR"); polytope > print join("\n ", $S>[0]>list); precondition : N_RAYS  N_INPUT_RAYS ( ppl.convex_hull.primal: FACETS, LINEAR_SPAN : RAYS  INPUT_RAYS ) sensitivity check for FacetPerm ... # output as expected polytope > $L = []; $L>[0] = $S>[0]>list; polytope > print join("\n ", $L>[0]); 15
WTF??
I suppose the problem here is that in the line $L>[0] = $S>[0]>list;
perl puts a scalar value into the first entry of the list that L points to. In contrast to what I thought perl would do, it does not put a reference (which is a scalar) to $S>[0]>list
into the first entry of L, but it puts there a scalar interpretation of the array $S>[0]>list
 which is the length of that array.
Crap.
comment:22 Changed 6 years ago by
I will now try this model:
 A variable created by Sage in the interface are arrays.
 Such variable either corresponds to a genuine Perl array, or it is an array of length one whose single entry is a reference to some Polymake object.
 I only need to distinguish one thing: If var is an array of length>0, then it is guaranteed to be a genuine Perl array, and we consistently use var as
@var
. Otherwise, we use it in the form$var[0]
.  Since the length of the array is determined at the time of its creation, I can store an attribute in the Sage wrapper that tells what case we'll have to deal with.
 As far as I can see, the only problem would be a genuine Perl array x of length 1, that we would mistakenly interpret as a Polymake object wrapped in x; this would only be problematic if the array x would later be modified.
Anyway. I will next try to use this approach to assign expr
in Polymake to a variable:
@SAGE0 = (expr);
, and"SAGE0"
is the autogenerated name of a Sage wrapper for expr. Test the length of
@SAGE0
. If the length is 1, then the sage wrapper stores the information that to access the wrapped value it has to use$SAGE0[0]
. Otherwise, it stores the information that to access the wrapped value it has to use@SAGE0
.  Do not care about the case that
expr
is an array of length one.
comment:23 Changed 6 years ago by
Commit:  06e85fd9d84b15f06c4aa2e14d4db84c009c0459 → 69e01f35fc42e27b2b6f3b01357c84ff3fb0a2ad 

Branch pushed to git repo; I updated commit sha1. New commits:
e801cab  Change to a different approach to wrapping Perl variables

87ad586  Change syntax for catching 'ValueError as...'

fec57d1  Merge branch 't/22501/make_it_easier_to_customize_pexpect_interfaces' into t/22452/create_a_polymake_pexpect_interface

69e01f3  Small change to how certain element types are printed

comment:24 Changed 6 years ago by
I have implemented the approach explained in the previous comment.
Here I show that I can basically recreate a rather nontrivial computation from this polymake tutorial:
sage: from sage.interfaces.polymake import polymake sage: p = polymake.cube(3) sage: set_verbose(3) sage: p.N_LATTICE_POINTS used package latte LattE (Lattice point Enumeration) is a computer software dedicated to the problems of counting lattice points and integration inside convex polytopes. Copyright by Matthias Koeppe, Jesus A. De Loera and others. http://www.math.ucdavis.edu/~latte/ 27 sage: p.HILBERT_BASIS_GENERATORS used package 4ti2 4ti2  A software package for algebraic, geometric and combinatorial problems on linear spaces. Copyright by 4ti2 team. http://www.4ti2.de/ <1 0 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 0 0 1 1 1 0 1 1 1 0 1 1 0 1 1 1 1 1 1 1 0 1 0 1 1 1 0 1 1 1 0 1 1 1 0 1 0 0 0 1 1 0 0 1 1 0 0 1 0 1 0 1 1 1 0 1 0 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 0 0 1 1 1 0 1 1 1 0 1 1 0 1 1 > <>
I am not totally happy with the string representation of this, but I guess it is still nice enough. Note that a different package is used compared with what is stated in the tutorial. Probably this is because of a different polymake version:
sage: set_verbose(0) sage: polymake.version() '3.0'
whereas the tutorial uses version 2.9.6.
With the pexpect interface, it is also possible to create a new instance of a Polymake class, but the typical name ("new
") is not available, as a method with this name but with a different semantics exists in Sage's interface framework. So, I chose a different name:
sage: q = polymake.new_from_type("Polytope", INEQUALITIES=[[5,4,0,1],[3,0,4,1],[2,1,0,0],[4,4,4,1],[0,0,1,0],[8,0,0,1],[1,0,1,0],[3,1,0,0]]) sage: q Polytope<Rational>[SAGE45]
Again, the string representation could be subject of discussion. The current proposal is to take into account the type name and the name of the Sage variable representing the instance.
sage: set_verbose(3) sage: q.VERTICES used package ppl The Parma Polyhedra Library (PPL): A C++ library for convex polyhedra and other numerical abstractions. http://www.cs.unipr.it/ppl/ 1 3 0 8 1 2 1 8 1 3 1 8 1 2 0 3 1 3 0 7 1 3 1 7 1 2 1 7 1 2 0 4 sage: q.NORMAL sage: q.HILBERT_BASIS_GENERATORS <1 3 0 8 2 5 1 13 1 2 1 8 1 2 0 3 1 3 0 7 1 2 1 7 1 3 1 7 1 3 1 8 1 2 0 4 > <> sage: q.VERY_AMPLE 1
I think the order of facets has changed between versions, since some of the following differs from what is in the tutorial:
sage: f = q.facet(2) sage: f.VERY_AMPLE 1 sage: f.VERTICES 1 2 1 8 1 2 0 3 1 2 1 7 1 2 0 4 sage: q.FACETS 5 4 0 1 3 0 4 1 2 1 0 0 4 4 4 1 0 0 1 0 8 0 0 1 1 0 1 0 3 1 0 0
Here I show that Polymake's warnings are taken care of (and so are errors):
sage: f.FACET_WIDTH /home/king/Sage/git/sage/local/lib/python2.7/sitepackages/sage/interfaces/polymake.py:464: RuntimeWarning: could not compute 'FACET_WIDTH' probably because of unsatisfied preconditions: precondition : FULL_DIM ( FACET_WIDTHS : VERTICES , FACETS , CONE_AMBIENT_DIM ) warnings.warn(w, RuntimeWarning) sage: g = f.ambient_lattice_normalization() sage: q.FACET_WIDTH 5 sage: cr = p.vertex_lattice_normalization() sage: cr.VERTICES (4) (0 1) 1 1 0 0 1 0 1 0 1 1 1 0 1 0 0 1 1 1 0 1 1 0 1 1 1 1 1 1
In polymake, you sometimes need to load a different application in order to work. In terms of the pexpect interface, this is a nontrivial change, as the current prompt depends on the current application. Nonetheless, I made it work:
sage: polymake.application('fan') sage: f = p.normal_fan() sage: f.SMOOTH_FAN 1 sage: g = q.normal_fan() sage: g.RAYS 1 0 1/4 0 1 1/4 1 0 0 1 1 1/4 0 1 0 0 0 1 0 1 0 1 0 0 sage: g.RAYS.primitive() 4 0 1 0 4 1 1 0 0 4 4 1 0 1 0 0 0 1 0 1 0 1 0 0 sage: g.MAXIMAL_CONES {3 4 5 7} {2 3 5 6} {5 6 7} {0 1 2 4} {0 4 7} {0 1 6 7} {1 2 6} {2 3 4} sage: g.HASSE_DIAGRAM.FACES.rows_numbered() 0: 1:0 2:1 3:2 4:3 5:4 6:5 7:6 8:7 9:0 1 10:0 4 11:0 7 12:1 2 13:1 6 14:2 3 15:2 4 16:2 6 17:3 4 18:3 5 19:4 7 20:5 6 21:5 7 22:6 7 23:3 4 5 7 24:2 3 5 6 25:5 6 7 26:0 1 2 4 27:0 4 7 28:0 1 6 7 29:1 2 6 30:2 3 4 31:0 1 2 3 4 5 6 7 sage: g.HASSE_DIAGRAM.nodes_of_dim(2) {9 10 11 12 13 14 15 16 17 18 19 20 21 22}
Visualisation does not work. I don't know if that can be achieved with a pexpect interface.
TODO_
Add documentation. Apart from that, I currently see no obvious issue.
New commits:
e801cab  Change to a different approach to wrapping Perl variables

87ad586  Change syntax for catching 'ValueError as...'

fec57d1  Merge branch 't/22501/make_it_easier_to_customize_pexpect_interfaces' into t/22452/create_a_polymake_pexpect_interface

69e01f3  Small change to how certain element types are printed

comment:25 Changed 6 years ago by
Commit:  69e01f35fc42e27b2b6f3b01357c84ff3fb0a2ad → 054123b0e98db7040fa83682283130e4fff62dc9 

Branch pushed to git repo; I updated commit sha1. New commits:
054123b  Extending a method to use 'new' for Polymake classes

comment:26 followup: 29 Changed 6 years ago by
I think I will change the strategy for wrapping yet again.
Basic problem to solve
In Perl, the different basic types HASH, ARRAY, SCALAR, and IO require a different syntax in order to perform "equivalent" operations on them. Even to assign some data X to a variable, the required syntax depends on the type of X. So, if I don't know in advance the type of some data X then there's your problem.
My current approach is to store all data in an ARRAY: If the data X is in fact an ARRAY then the wrap is the same as X, as Perl doesn't know arrays of arrays; otherwise the wrap is a list of length one containing X. So, assigning data X to a wrapper variable W, I know how to do so independent of the type of X. For all subsequent operations on X, I can test the type of the first entry of W (if len(W)==1) respectively I know that X is the same as W, and then know what to do.
Caveat: My current approach would fail if X happens to be an array of length 1.
I think a cleaner solution would be this: Write a little Perl function get_type_of
, such that get_type_of(X)
returns the array ("ARRAY", X)
resp. ("HASH", X)
resp. ("SCALAR", X)
depending on the type of X  this could probably be done by some "try  catch" magic.
Now, when wrapping a polymake object in the pexpect interface, the output of get_type_of
would be used to determine whether we need to evaluate $SAGE1 = X;
or @SAGE1 = X;
or %SAGE1 = X;
to create the wrapper. Of course, the type would be stored, so that in all later operations it is clear whether, for example, item access is done be @SAGE1[14]
or %SAGE1{14}
.
But this has to wait for later, as I should be busy with different things today.
comment:27 Changed 6 years ago by
Commit:  054123b0e98db7040fa83682283130e4fff62dc9 → 15b6cc65c7415656df8881214ea6eb4720e6dc74 

Branch pushed to git repo; I updated commit sha1. New commits:
15b6cc6  Remove automated creation of a logfile

comment:28 Changed 6 years ago by
Commit:  15b6cc65c7415656df8881214ea6eb4720e6dc74 → b9009929e066bf5c002836702ab968bbc098ba9d 

Branch pushed to git repo; I updated commit sha1. New commits:
b900992  fix a bug in getting docs from polymake

comment:29 Changed 6 years ago by
Replying to SimonKing:
I think I will change the strategy for wrapping yet again. ... I think a cleaner solution would be this: Write a little Perl function
get_type_of
, such thatget_type_of(X)
returns the array("ARRAY", X)
resp.("HASH", X)
resp.("SCALAR", X)
depending on the type of X  this could probably be done by some "try  catch" magic.
I tried, and it seems it won't work. At least it won't work better than the current approach. So, I'll keep it.
Andreas Paffenholz has just pointed out to me how to parse the output of apropos "";
in order to not only get the functions of the current application, but also to get member and member function names, which is important for tab completion respectively for knowing what a Sage user means when she types X.name_of_something
: An property of X? A polymake function that, when called, is applied to X and further given arguments? A member function of X that, when called, is evaluated only on the further given arguments?
Also I made some experiments with KeyboardInterrupt. Sometimes I found that it crashed the interface (i.e., polymake._expect
became None), and so far I don't understand why this can happen, and why it doesn't always happen.
comment:30 Changed 6 years ago by
Dependencies:  → 22501 

comment:31 Changed 6 years ago by
Dependencies:  22501 → #22501 

comment:32 Changed 6 years ago by
In addition to the problem with keyboard interrupt, I got this problem (probably related):
sage: from sage.interfaces.polymake import polymake sage: p = polymake.rand_sphere(4,23, seed=5) sage: t = p.TRIANGULATION sage: t? Traceback (most recent call last): ... RuntimeError: Polymake terminated unexpectedly while reading in a large line: unknown help topic 'Polymake::topaz::GeometricSimplicialComplex__Rational::__as__Polymake::polytope::Polytope::_prop_TRIANGULATION'
Andreas and Julian have told me a bit more about several kinds of type names, and it seems they would be useful both to access the documentation and to get a more accurate list of available properties and member functions.
comment:33 Changed 6 years ago by
A new complication arose: So far, the patterns I was looking for with pexpect have been application_name >
(normal prompt), application_name (digits) >
(that's the continuation prompt), killed by signal
, polymake: ERROR:
, and polymake: WARNING
.
But I have just learned that when asking for help "Triangulation"
there is yet another prompt to take care of:
polytope > help "TRIANGULATION"; There are 5 help topics matching 'TRIANGULATION': 1: objects/Cone/properties/Triangulation and volume/TRIANGULATION 2: objects/Polytope/properties/Triangulation and volume/TRIANGULATION 3: objects/Visualization/Visual::PointConfiguration/methods/TRIANGULATION 4: objects/Visualization/Visual::Polytope/methods/TRIANGULATION 5: objects/PointConfiguration/properties/Triangulation and volume/TRIANGULATION  Please choose those interesting you via history navigation (ArrowUp/ArrowDown): polytope [1]>
I was told by the polymake developers at !SageDays84 that the prompt with square brackets will only arise when requesting help.
So, what shall we do in such situation? Display the available information (using _sage_doc_
) and leave it to the user to access the different help topics? Always choose the first available help option? Other ideas?
comment:34 Changed 6 years ago by
I guess the easiest would be to generally look for the application_name [digits] >
pattern and send a chr(3)
when it occurs, as this should bring back the normal prompt. And then return the output on top of the pattern. So,that would be the solution "leave it to the user to access the different help topics". And _sage_doc_
could in that case simply return the empty string (or maybe the string representation of the Polymake type).
comment:35 Changed 6 years ago by
Commit:  b9009929e066bf5c002836702ab968bbc098ba9d → 3e5b32df91d72075ee5470c22282ca93c828abf9 

Branch pushed to git repo; I updated commit sha1. New commits:
d9e2b6f  Use a more recommended way to get a description of a big object in Polymake

63418a6  Fix that when requesting help, Polymake may request user interrupt.

54ea91c  Merge branch 'develop' into t/22452/create_a_polymake_pexpect_interface

3e5b32d  Add some documentation for the Polymake interface

comment:36 Changed 6 years ago by
Commit:  3e5b32df91d72075ee5470c22282ca93c828abf9 → 6784d6cd2ba194ee259f72e179e8287e0dddd5a5 

Branch pushed to git repo; I updated commit sha1. New commits:
6784d6c  Full doctest coverage for Polymake pexpect interface

comment:37 Changed 6 years ago by
Commit:  6784d6cd2ba194ee259f72e179e8287e0dddd5a5 → 8485ec834b7d3d03790c821ea06a4f4415ae56e4 

Branch pushed to git repo; I updated commit sha1. New commits:
8485ec8  Be slightly clearer concerning a test being skipped

comment:38 Changed 6 years ago by
Authors:  → Simon King 

Status:  new → needs_review 
The last commits provide full doctest coverage and passes (on my machine) both without polymake (then running sage t
) and with polymake (then running sage t optional=sage,polymake
).
Caveats
 I assume in the tests that the Polymake version is what is currently available by
sage i polymake
: It is version 3.0. Hopefully that's fine.  The test for
_keyboard_interact
is quite flaky. Interactively, it sometimes results in a warning, sometimes in an AttributeError, sometimes it just works as stated. I have no clue concerning the reasons. Anyway, I marked it as "not tested".  In the test for
_sage_doc_
, I mention another thing I don't understand: When I dosage: p = polymake.rand_sphere(3,13, seed=12) sage: p.get_schedule?
one does not get the Polymake documentation ofget_schedule
, even thoughsage: print p.get_schedule._sage_doc_() objects/Core::Object/methods/get_schedule: get_schedule(request; ... ) > Core::RuleChain Compose an optimal chain of production rules providing all requested properties. The returned RuleChain object can be applied to the original object as well as to any other object with the same initial set of properties. If no feasible rule chain exists, `undef' is returned. To watch the rule scheduler at work, e.g. to see announcements about tried preconditions, you may temporarily increase the verbosity levels $Verbose::rules and $Verbose::scheduler. Arguments: String request : name of a property with optional alternatives or a property path in dotted notation. Several requests may be listed. Returns Core::RuleChain
does the right thing. In contrast,sage: p.minkowski_sum_fukuda?
just works. Strange.
Anyway, I think it is good enough for being reviewed!
New commits:
8485ec8  Be slightly clearer concerning a test being skipped

comment:39 Changed 6 years ago by
Reviewers:  → Vincent Delecroix 

I am going through the changes right now. I will check compatibility with the last beta of polymake as well and report.
comment:40 Changed 6 years ago by
Thank you for testing.
The failures, as much as I see, are all harmless in the sense that simply the string representation of an object was enriched or the documentation was changed or Polymake's strategy for computing a property has changed.
What is the policy? Should the doctests for an optional package be written with respect to what you would get from the current version of the optional package, or should be "future proof" to all possible extent?
comment:41 followup: 48 Changed 6 years ago by
Status:  needs_review → needs_work 

first pass
 The program is
polymake
and notPolymake
 need an entry in the reference manual
src/doc/en/reference/interfaces/index.rst
 your method
version
is somehow broken. You are using the programpolymake
and not the command that was transmitted by the user. Andreas suggestedpolytope > print $Polymake::Version; 3.0.9
 you use twice
os.getenv('SAGE_POLYMAKE_COMMAND') or 'polymake'
. One possibility would be to set a variable namedSAGE_POLYMAKE_COMMAND
at the module level once for all and use it all along.
 the install hints should give more details about polymake installation (e.g. the SPKG.txt contains some subtle details)
 in the doctest of
application
I would suggest to not quit and start but ratherEXAMPLES:: sage: polymake.application('polytope') sage: 'tubing_of_graph' in dir(polymake) False sage: polymake.application('tropical') sage: 'tubing_of_graph' in dir(polymake) True sage: polymake.application('polytope') sage: 'tubing_of_graph' in dir(polymake) False
 (just a remark) polymake namespaces are strange...
sage: apps = ['polytope', 'matroid', 'graph', 'group', 'topaz', 'tropical'] sage: for app in apps: ....: polymake.application(app) ....: dirs[app] = dir(polymake) sage: for app1 in apps: ....: for app2 in apps: ....: if app1 != app2: print "{:10} {:10} {}".format(app1, app2, len(set(dirs[app1]).difference(dirs[app2]))) ....: polytope matroid 0 polytope graph 345 polytope group 338 polytope topaz 278 polytope tropical 0 matroid polytope 61 matroid graph 406 matroid group 399 matroid topaz 339 matroid tropical 0 graph polytope 0 graph matroid 0 graph group 34 graph topaz 0 graph tropical 0 group polytope 0 group matroid 0 group graph 41 group topaz 41 group tropical 0 topaz polytope 0 topaz matroid 0 topaz graph 67 topaz group 101 topaz tropical 0 tropical polytope 192 tropical matroid 131 tropical graph 537 tropical group 530 tropical topaz 470
 in
application
you should check errors likesage: polymake.application('killerapp') Traceback (most recent call last): ... AssertionError: Unknown Polymake application 'killerapp'
and BTW the above would better be aValueError
comment:42 Changed 6 years ago by
You would better not import Polymake
(upper case) in the global namespace.
comment:43 Changed 6 years ago by
You could test something like this with respect to the version
sage: p = Polymake(command='/ThIs/iS/NoT/PoLyMaKe') sage: p.version() # this should raise an error '3.0.6'
comment:44 followup: 47 Changed 6 years ago by
I found the method console
confusing. It launches polymake
but with a fresh session. I would expect to use the same console with the same variables already defined. Apparently it is a standard bad practice.
comment:45 followup: 49 Changed 6 years ago by
Is this really what I need to use polymake.get('$myvar[0]')
. Why not the cleaner polymake.get('myvar')
?
comment:46 Changed 6 years ago by
 There is a
Math Processing Error
appearing in the HTML documentation of the methodshelp
andget_member
.
 (bike shedding) The line
sage: p = polymake.rand_sphere(4,20, seed=5)
should besage: p = polymake.rand_sphere(4, 20, seed=5)
comment:47 Changed 6 years ago by
Replying to vdelecroix:
Apparently it is a standard bad practice.
Indeed. At least I do not follow another bad practice, which is to put all consoles into the global name space, even though there is the .console() method.
By the way, there is an .interact() method for the thing you want to do.
comment:48 Changed 6 years ago by
Replying to vdelecroix:
first pass
 The program is
polymake
and notPolymake
OK. I think one should keep the string representation of the interface instance upper case (which is consistent with everything but pari/gp).
 your method
version
is somehow broken. You are using the programpolymake
and not the command that was transmitted by the user. Andreas suggestedpolytope > print $Polymake::Version; 3.0.9
Thanks, I wasn't aware of that constant.
 the install hints should give more details about polymake installation (e.g. the SPKG.txt contains some subtle details)
OK, I'll add: "(but read its SPKG.txt first!)"
 in the doctest of
application
I would suggest to not quit and start but ratherEXAMPLES:: sage: polymake.application('polytope') sage: 'tubing_of_graph' in dir(polymake) False sage: polymake.application('tropical') sage: 'tubing_of_graph' in dir(polymake) True sage: polymake.application('polytope') sage: 'tubing_of_graph' in dir(polymake) False
Good idea! I asked Andreas about such examples, but he only came up with the thing that I used in the previous versions of the test.
 in
application
you should check errors likesage: polymake.application('killerapp') Traceback (most recent call last): ... AssertionError: Unknown Polymake application 'killerapp'and BTW the above would better be aValueError
OK
comment:49 Changed 6 years ago by
Replying to vdelecroix:
Is this really what I need to use
polymake.get('$myvar[0]')
. Why not the cleanerpolymake.get('myvar')
?
No. I tried to explain it in the NOTE on top of that test. But in the next commit that I am about to push, I'll try to be clearer in the NOTE.
comment:50 Changed 6 years ago by
Commit:  8485ec834b7d3d03790c821ea06a4f4415ae56e4 → afdbcdb19408f1837b78a2233c0989037633cee8 

Branch pushed to git repo; I updated commit sha1. New commits:
afdbcdb  Taking into account the reviewer's comments

comment:51 Changed 6 years ago by
Status:  needs_work → needs_review 

Does work with my version of polymake, and I hope it works with the next beta version as well.
Back to "needs review" :)
New commits:
afdbcdb  Taking into account the reviewer's comments

Changed 6 years ago by
Attachment:  doctest22452polymake3.0.6.log added 

doctest at commit afdbcdb for polymake 3.0.6
Changed 6 years ago by
Attachment:  doctest22452polymake3.0.9.log added 

doctest at commit afdbcdb for polymake 3.0.9
comment:52 Changed 6 years ago by
(note: The tests on beta 3.0.6 and 3.0.9 releases do not fail in the same way)
comment:53 followup: 54 Changed 6 years ago by
Status:  needs_review → needs_work 

This is still wrong (see also point 4 in comment:41)
sage: from sage.interfaces.polymake import Polymake sage: p = Polymake(command='haha') sage: p.console() Welcome to polymake version 3.0.6 ...
comment:54 Changed 6 years ago by
Replying to vdelecroix:
This is still wrong (see also point 4 in comment:41)
sage: from sage.interfaces.polymake import Polymake sage: p = Polymake(command='haha') sage: p.console() Welcome to polymake version 3.0.6 ...
I disagree.
sage: m = Magma(command="foobar") sage: m.console() /home/king/Sage/git/sage/local/bin/sagenativeexecute: 6: /home/king/Sage/git/sage/local/bin/sagenativeexecute: magma: not found
So, apparently at least one other interface is doing the same thing (it says "magma: not found" where you would expect "foobar: not found").
comment:55 Changed 6 years ago by
Andreas created functions that help to get lists of properties, functions and member functions, and also tab completion. These functions will be in future polymake versions.
It would of course be very good if we could backport it, so that it can be used now (without the need to wait for an upgrade of the optional polymake package in Sage).
The approach would be:
 Put Andreas' functions in a script, located in sage/ext_code
 When starting the polymake interface, check whether Andreas' functions are already defined (which means that the user has a new polymake version)
 If they are not defined, read the script into polymake and thus have tab completion more easily.
comment:56 Changed 6 years ago by
Commit:  afdbcdb19408f1837b78a2233c0989037633cee8 → 9a1e5f50e82a132513758f011a609803e33ae83c 

Branch pushed to git repo; I updated commit sha1. New commits:
4bb46e0  Let the .console() method raise a NotImplementedError with pointers to .interact() and polymake_console()

9a1e5f5  Remove test that accesses a member that soon will be removed from Polymake. Mark tests requesting documentation 'random'

comment:57 Changed 6 years ago by
Commit:  9a1e5f50e82a132513758f011a609803e33ae83c → 7892e302c06bc518b4e858ac377dd76f1c5bdfe1 

Branch pushed to git repo; I updated commit sha1. New commits:
7892e30  Replace some tests by more realistic use cases of Polymake, and mark some others as 'random'

comment:58 Changed 6 years ago by
Status:  needs_work → needs_review 

In the last few commits, I did the following:
 When showing in the tests how to request documentation, I give the whole output as provided by polymake (which should be instructive), but mark the corresponding test as random, as the polymake documentation is of course subject to change in future versions.
 Similar, the member function
get_schedule
yields output that is subject to change. Moreover,get_schedule
is not something that one normally uses in polymake. So, I was marking some of the corresponding tests "random", and replaced others (in particular those that are visible in the docs) by more realistic use cases ("check whether some point is contained in a polytope").  Remove a test in which a member of
GROUP
is requested that will soon be gone.
For me (polymake3.0) tests pass (both sage t optional=sage,polymake
and plain sage t
). And for you, Vincent?
comment:60 followup: 62 Changed 6 years ago by
Status:  needs_review → needs_work 

Input
should be INPUT
. This might be the cause of the weird display of the documentation of the method Polymake.application
comment:61 Changed 6 years ago by
Commit:  7892e302c06bc518b4e858ac377dd76f1c5bdfe1 → a18537b66b9ee27475c96268b0491fe89c8c3677 

Branch pushed to git repo; I updated commit sha1. New commits:
a18537b  Change Input to INPUT

comment:62 Changed 6 years ago by
Status:  needs_work → needs_review 

Replying to vdelecroix:
Input
should beINPUT
. This might be the cause of the weird display of the documentation of the methodPolymake.application
Done! And good that the tests even pass with the latest polymakebeta :)
comment:63 followups: 64 65 Changed 6 years ago by
Status:  needs_review → needs_work 

 The link
:meth:`~sage.interfaces.Interface.interact`
in the methodconsole
is not working.
 The display of the method
application
is still wrong. I think it is not safe to have a linebreak before::
. Anyway, it would be good to haveEXAMPLES
sinceTESTS
do not appear in the doc. Something likesage: polymake.application("app1"). sage: polymake.a_function_in_app1() sage: polymake.application("app2") sage: polymake.a_function_in_app2()
 Is
Evaluate of a command
proper english (in the doc of_eval_line
)?
 Links in hidden methods do not appear in the doc (like
_eval_line
). You can either make this method appear with some sphinx directive or remove the links.
if not wait_for_prompt
is better thanif wait_for_prompt == False
.elif restart_if_needed
better thanelif restart_if_needed==True
.
 It is hard to see that all code paths are actually tested in
_eval_line
. Is it reasonable to have 8 small tests that actually do that?
comment:64 followup: 68 Changed 6 years ago by
The patchbot complains about print statements in the doc tests. So, shall I replace all print statements by print function? Would that require from __future__ import
?
Replying to vdelecroix:
 The link
:meth:`~sage.interfaces.Interface.interact`
in the methodconsole
is not working.
Do you see why it doesn't?
 The display of the method
application
is still wrong. I think it is not safe to have a linebreak before::
.
I thought that is what one ought to do if one doesn't like a colon to appear before a test. But I can change that.
Anyway, it would be good to have
EXAMPLES
sinceTESTS
do not appear in the doc.
They don't? Ouch. I thought they used to appear in the past.
Something like
sage: polymake.application("app1"). sage: polymake.a_function_in_app1() sage: polymake.application("app2") sage: polymake.a_function_in_app2()
Sure. In Olot, I presented an example from the polymake tutorial that I could use here.
 Is
Evaluate of a command
proper english (in the doc of_eval_line
)?
Probably not...
 Links in hidden methods do not appear in the doc (like
_eval_line
). You can either make this method appear with some sphinx directive or remove the links.
OK.
if not wait_for_prompt
is better thanif wait_for_prompt == False
.elif restart_if_needed
better thanelif restart_if_needed==True
.
OK. Note that I copied that from some other module (sage.interfaces.interface or sage.interfaces.expect). But I can change it anyway.
 It is hard to see that all code paths are actually tested in
_eval_line
. Is it reasonable to have 8 small tests that actually do that?
I think timeout and eof isn't tested, but everything else is tested. But not all options are tested in _eval_line
(e.g., polymake waiting for user input is tested in help()
). Anyway I agree it makes sense to test it directly in _eval_line
.
comment:65 Changed 6 years ago by
Replying to vdelecroix:
 Links in hidden methods do not appear in the doc (like
_eval_line
). You can either make this method appear with some sphinx directive or remove the links.
What would I need to do in order to make it visible in the docs? I think it SHOULD be visible.
I'll also include a test showing how to use a timeout. In fact, I needed a fix in order to make it work.
comment:66 followup: 69 Changed 6 years ago by
It seems that if the same warning is raised twice, only the first is shown. Thus, if I want to add a test in _eval_line
that does the same as the test in help
, then the warning is not shown.
How to prevent this?
comment:67 Changed 6 years ago by
I found out about .. automethod::
, and I think in the "warning" issue, it is fine to mark it as "random", since the output will change with future polymake versions anyway. The main point is that the interface doesn't freeze, although polymake expects user interaction.
comment:68 followup: 71 Changed 6 years ago by
Replying to SimonKing:
The patchbot complains about print statements in the doc tests. So, shall I replace all print statements by print function? Would that require
from __future__ import
?
No, it only works at module level. We are in a crazy transition period where we want to use doctests compatible with both Python2 and Python3. So always write print(x)
and avoid print(x,y)
.
Replying to vdelecroix:
 The link
:meth:`~sage.interfaces.Interface.interact`
in the methodconsole
is not working.Do you see why it doesn't?
There is no such object sage.interfaces.Interface.interact
.
Anyway, it would be good to have
EXAMPLES
sinceTESTS
do not appear in the doc.They don't? Ouch. I thought they used to appear in the past.
In the past yes ;)
comment:69 followup: 70 Changed 6 years ago by
Replying to SimonKing:
It seems that if the same warning is raised twice, only the first is shown. Thus, if I want to add a test in
_eval_line
that does the same as the test inhelp
, then the warning is not shown.How to prevent this?
You need to set the warnings to "always"
instead of "once"
I am not sure it is safe to modify this configuration in doctests but it is worth giving a try.
comment:70 Changed 6 years ago by
Replying to vdelecroix:
Replying to SimonKing:
It seems that if the same warning is raised twice, only the first is shown. Thus, if I want to add a test in
_eval_line
that does the same as the test inhelp
, then the warning is not shown.How to prevent this?
You need to set the warnings to
"always"
instead of"once"
I am not sure it is safe to modify this configuration in doctests but it is worth giving a try.
Since the output *after* the warning is random anyway (as it will change with future versions of polymake), I guess it is safe to mark the test as random. We only want to see that the interface doesn't hang and doesn't raise an error.
comment:71 Changed 6 years ago by
Replying to vdelecroix:
Replying to vdelecroix:
 The link
:meth:`~sage.interfaces.Interface.interact`
in the methodconsole
is not working.Do you see why it doesn't?
There is no such object
sage.interfaces.Interface.interact
.
That's a compelling reason!
Anyway, it would be good to have
EXAMPLES
sinceTESTS
do not appear in the doc.They don't? Ouch. I thought they used to appear in the past.
In the past yes ;)
I changed some TESTS in EXAMPLES and I think in one case EXAMPLES to TEST. Commit coming soon...
comment:72 Changed 6 years ago by
Commit:  a18537b66b9ee27475c96268b0491fe89c8c3677 → f718010e5af55cfdd71810cbc92374e6b5a14ef6 

comment:73 Changed 6 years ago by
Commit:  f718010e5af55cfdd71810cbc92374e6b5a14ef6 → 0e598ac2f533471cca6fce8728de8e2ea4d6a735 

Branch pushed to git repo; I updated commit sha1. New commits:
0e598ac  Make it clearer that 'set()' is an internal function. Make reaction on interrupt more stable

comment:74 Changed 6 years ago by
Status:  needs_work → needs_review 

I added a test showing how to set a timeout (this required a fix).
However, I am not totally happy. While the tests related with timeout and keyboard interrupt mostly (but not always) work, they quite often fail in the doctests. I have no idea why, but I think this can wait for a subsequent ticket, as I believe the interfaces generally works.
Further ideas for the future: Implement pickling.
But for now, I think it is "needs review". The docs seem better, and also I made it clearer what methods are internal and how the interface is supposed to be used.
comment:75 Changed 6 years ago by
Status:  needs_review → needs_work 

$ SAGE_POLYMAKE_COMMAND='NOOP' sage t long optional=sage polymake.py Running doctests with ID 2017031320381528caf477. Git branch: u/SimonKing/create_a_polymake_pexpect_interface Using optional=sage Doctesting 1 file. sage t long polymake.py ********************************************************************** File "polymake.py", line 648, in sage.interfaces.polymake.Polymake.set Failed example: c == d Exception raised: Traceback (most recent call last): ... NameError: name 'c' is not defined ********************************************************************** File "polymake.py", line 653, in sage.interfaces.polymake.Polymake.set Failed example: c.VERTICES == d.VERTICES Exception raised: Traceback (most recent call last): ... NameError: name 'c' is not defined ********************************************************************** File "polymake.py", line 684, in sage.interfaces.polymake.Polymake.get Failed example: polymake('cube(3)') Exception raised: Traceback (most recent call last): ... TypeError: unable to start polymake ********************************************************************** 2 items had failures: 1 of 2 in sage.interfaces.polymake.Polymake.get 2 of 3 in sage.interfaces.polymake.Polymake.set [24 tests, 3 failures, 0.23 s]  sage t long polymake.py # 3 doctests failed  Total time for all tests: 5.3 seconds cpu time: 0.2 seconds cumulative wall time: 0.2 seconds
comment:76 Changed 6 years ago by
However, this is fine
$ sage t long optional=sage,polymake polymake.py sage t long polymake.py [218 tests, 20.41 s]  All tests passed! 
comment:77 followup: 79 Changed 6 years ago by
There are two [Math Processing Error]
in the method application
.
comment:78 Changed 6 years ago by
There is a warning block in the documentation. Did you mean .. WARNING::
? (see dev. guide)
comment:79 Changed 6 years ago by
Replying to vdelecroix:
There are two
[Math Processing Error]
in the methodapplication
.
Where do you see such errors? When I tried to search for it in the logs, I couldn't find them, and the documentation looks fine.
comment:80 Changed 6 years ago by
Commit:  0e598ac2f533471cca6fce8728de8e2ea4d6a735 → b2fd6df9e6975bb2f4b8ccbd133da97ebd4f9954 

Branch pushed to git repo; I updated commit sha1. New commits:
b2fd6df  Mark three tests as optional and create a proper WARNING block.

comment:81 followup: 82 Changed 6 years ago by
The failing doctests should be marked optional. I created a .. WARNING::
block properly. And I need more information concerning the [Math Processing Error]
you found.
New commits:
b2fd6df  Mark three tests as optional and create a proper WARNING block.

comment:82 Changed 6 years ago by
Replying to SimonKing:
And I need more information concerning the
[Math Processing Error]
you found.
I think it is fine. My Mathjax installation was doing something bad.
comment:83 Changed 6 years ago by
Status:  needs_work → positive_review 

Good to go! Thanks for your hard work.
We shoud think about moving polymake
into optional. I will open a poll on sagedevel
. The main advantage is that it will be tested by default on my patchbot.
comment:84 Changed 6 years ago by
Status:  positive_review → needs_work 

Oups. Need to wait for #22501. What is the status there?
comment:85 Changed 6 years ago by
Thanks a lot Simon! Thanks a lot Vincent! I look forward to test the interface! ...once my polymake compiles...
comment:86 Changed 6 years ago by
Trying to interactively rebase this on top of #22501, I get
king@kingThinkPadT430:~/Sage/git/sage$ git rebase i t/22501/make_it_easier_to_customize_pexpect_interfaces_new The previous cherrypick is now empty, possibly due to conflict resolution. If you wish to commit it anyway, use: git commit allowempty Otherwise, please use 'git reset' interactive rebase in progress; onto 5082c96 Last commands done (7 commands done): pick e801cab Change to a different approach to wrapping Perl variables pick 87ad586 Change syntax for catching 'ValueError as...' Next commands to do (18 remaining commands): pick 69e01f3 Small change to how certain element types are printed pick 054123b Extending a method to use 'new' for Polymake classes You are currently rebasing branch 't/22452/create_a_polymake_pexpect_interface' on '5082c96'. nothing to commit, working directory clean Could not apply 87ad58632e42a3d0c75f7871391012fd8b328f35... Change syntax for catching 'ValueError as...'
So, several commits are now missing. Since git mergetool
reports that nothing needs to be merged, I don't know what to do. Please help!
comment:87 Changed 6 years ago by
Trac sucks! I keep getting logged out after being inactive for perhaps a minute.
Anyway, I solved the problem. I just did as instructed (allowed an empty commit) and then continued rebasing. Eventually I repeated git rebase i
so that I could remove the empty commit. Pushing the result now!
comment:88 Changed 6 years ago by
Commit:  b2fd6df9e6975bb2f4b8ccbd133da97ebd4f9954 → a8157a44cc528bccd16056a0b9ec6f5401cdd1ad 

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
3466f9c  Be slightly clearer concerning a test being skipped

b87433f  Taking into account the reviewer's comments

8f23735  Let the .console() method raise a NotImplementedError with pointers to .interact() and polymake_console()

b89f2c1  Remove test that accesses a member that soon will be removed from Polymake. Mark tests requesting documentation 'random'

f90647d  Replace some tests by more realistic use cases of Polymake, and mark some others as 'random'

bcbabe5  Change Input to INPUT

1238261  Fix a doc link. Fix dealing with timeouts. Demonstrate how to set timeouts

94460ae  Add further tests, fix doc formatting

643d6b4  Make it clearer that 'set()' is an internal function. Make reaction on interrupt more stable

a8157a4  Mark three tests as optional and create a proper WARNING block.

comment:89 followup: 90 Changed 6 years ago by
Status:  needs_work → needs_review 

Done! So, now it can be reviewed, modulo #22501. If I recall correctly, it is possible to give a review to a ticket even though not all dependencies have received a review. Or has that changed?
comment:90 Changed 6 years ago by
Replying to SimonKing:
Done! So, now it can be reviewed, modulo #22501. If I recall correctly, it is possible to give a review to a ticket even though not all dependencies have received a review. Or has that changed?
At some point, Volker complained about a ticket being positively reviewed but not some of its dependencies... (do not remember the ticket number though)
comment:91 Changed 6 years ago by
Status:  needs_review → needs_work 

Work issues:  → use print function in doctests 
Oops, I forgot to use print function instead of print statements in the doctests.
comment:92 Changed 6 years ago by
Commit:  a8157a44cc528bccd16056a0b9ec6f5401cdd1ad → 185430f80346564983f3aeee6a7ce5643c80d4d4 

Branch pushed to git repo; I updated commit sha1. New commits:
52226e2  Do not _check_valid in _repr_, since it is done in __repr__

726d3fd  Merge branch 't/22501/make_it_easier_to_customize_pexpect_interfaces_new' into t/22452/create_a_polymake_pexpect_interface

185430f  Use print function, not print statement, in doctests

comment:93 Changed 6 years ago by
Status:  needs_work → needs_review 

Work issues:  use print function in doctests 
The doctests are now using print function, not print statements. And for completeness, I have merged with the current version of #22501. Needs review, again.
comment:94 Changed 6 years ago by
Status:  needs_review → positive_review 

comment:95 Changed 6 years ago by
Status:  positive_review → needs_work 

sage t long src/sage/tests/py3_syntax.py ********************************************************************** File "src/sage/tests/py3_syntax.py", line 17, in sage.tests.py3_syntax Failed example: py3_syntax.run_tests('.py') # long time Expected nothing Got: Invalid Python 3 syntax found: File "src/sage/interfaces/polymake.py", line 1503 except PolymakeError, msg: ^ SyntaxError: invalid syntax <BLANKLINE> <BLANKLINE> **********************************************************************
comment:96 Changed 6 years ago by
Milestone:  sage7.6 → sage8.0 

comment:97 Changed 6 years ago by
Branch:  u/SimonKing/create_a_polymake_pexpect_interface → u/mkoeppe/create_a_polymake_pexpect_interface 

comment:98 Changed 6 years ago by
Commit:  185430f80346564983f3aeee6a7ce5643c80d4d4 → 52a77ee1b1fd70e0d6371f2789c4b6bfffe511ec 

Status:  needs_work → needs_review 
comment:99 Changed 6 years ago by
Status:  needs_review → positive_review 

comment:100 Changed 6 years ago by
Branch:  u/mkoeppe/create_a_polymake_pexpect_interface → 52a77ee1b1fd70e0d6371f2789c4b6bfffe511ec 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:101 Changed 6 years ago by
Commit:  52a77ee1b1fd70e0d6371f2789c4b6bfffe511ec 

Great!!!!! Thanks so much for all this work!!! I will let polymake people know and start showing it around!
As a follow up, I guess it would make sense to make a thematic tutorial on the interface?!?! I can volunteer to do it since I was playing a lot with the doc lately. I will continue to bug you with questions when I have them!
Thanks a lot again!
comment:102 Changed 6 years ago by
Indeed this is good news! I think it would be appropriate to show one example of a computation with polymake inside the thematic tutorial that you started at #22572. It can be a follow up ticket if you do not want to delay it.
Last 10 new commits:
Add documentation to _get_primitive
'Implement string representation by single underscore methods': Apply this mantra to interfaces
Change string representation of invalid interface elements whose parent is None
Keep the tests that have been removed in the previous commits
Parse error message of _check_valid() in order to cope with unpredictable future semantical changes
Include error message of validity check into string representation of invalid interface elements
Revert introduction of _get_custom_name
Actually remove _get_custom_name, not only its applications
Reverse addition of _get_primitive. Fix __cmp__, bool etc. and their documentation
Initial wrapper: providing help, tab completion, accessing members, calling functions, nice(r) string representation