#27965 closed enhancement (fixed)
py3: fix the last doctest in geometry
Reported by:  Frédéric Chapoton  Owned by:  

Priority:  major  Milestone:  sage8.9 
Component:  python3  Keywords:  
Cc:  JeanPhilippe Labbé, Vincent Delecroix, Travis Scrimshaw  Merged in:  
Authors:  Frédéric Chapoton, John Palmieri  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  6fcdf05 (Commits, GitHub, GitLab)  Commit:  6fcdf0594952b6aa3234a75ef0d7137c84655583 
Dependencies:  Stopgaps: 
Description
or rather tag it with # py2
I have no idea why this does fail only in python3. But anyway, we cannot really do anything with the snub dodecahedron currently.
Change History (12)
comment:1 Changed 3 years ago by
Branch:  → u/chapoton/27965 

Cc:  Travis Scrimshaw added 
Commit:  → 7c9a636fa9198ccf3af036ec59ba459db5664416 
Status:  new → needs_review 
comment:2 Changed 3 years ago by
I think the issue here is not that polytopes.snub_dodecahedron()
is broken in Python 3, but rather that Python 3 prints warning messages multiple times, whereas Python 2 only prints them once, at least while doctesting. A change like that in #27612 might be another option:
diff git a/src/sage/geometry/polyhedron/library.py b/src/sage/geometry/polyhedron/library.py index 71914d635a..c9bb1f35e8 100644  a/src/sage/geometry/polyhedron/library.py +++ b/src/sage/geometry/polyhedron/library.py @@ 1489,7 +1489,11 @@ class Polytopes(): Unfortunately, no polyhedra backend supports the construction of the snub dodecahedron at the moment::  sage: sd = polytopes.snub_dodecahedron() + sage: sd = polytopes.snub_dodecahedron() # py2 + sage: sd = polytopes.snub_dodecahedron() # py3 + doctest:warning + ... + UserWarning: This polyhedron data is numerically complicated; cdd could not convert between the inexact V and H representation without loss of data. The resulting object might show inconsistencies. sage: sd.f_vector() # not tested (1, 60, 150, 92, 1) sage: sd.base_ring() # not tested
Given the comment, though ("no polyhedra backend supports the construction of the snub dodecahedron at the moment"), it's not a big deal.
comment:3 Changed 3 years ago by
Branch:  u/chapoton/27965 → u/jhpalmieri/27965 

comment:4 Changed 3 years ago by
Authors:  Frédéric Chapoton → Frédéric Chapoton, John Palmieri 

Commit:  7c9a636fa9198ccf3af036ec59ba459db5664416 → 6fcdf0594952b6aa3234a75ef0d7137c84655583 
I prefer my suggestion. When we move away from Python 2, it leaves a valid py3 command to execute.
New commits:
6fcdf05  trac 27965: py3 fix in geometry/polyhedron/library.py

comment:5 Changed 3 years ago by
Reviewers:  → Frédéric Chapoton 

Status:  needs_review → positive_review 
ok, good for me.
comment:6 Changed 3 years ago by
... have a look at #25097.
This test was removed and replaced by a reasonable test using the backend normaliz.
I would say that such computation over RDF
should not be encouraged via examples/doctests as the backend normaliz provide reasonably fast computations with algebraic numbers.
comment:7 Changed 3 years ago by
cdd
and python2 and 3 have different behavior. In #27760, I was obtaining several times this kind of warnings in the default case due to approximation errors. When such errors occurred, I simply change the default behavior _not_ to produce such warnings and go for an exact solution.
I would really try to simply avoid making such tests in the future as we know that they are prone to produce such errors and we have a viable faster and reliable alternative (ok, with an optional package...).
comment:8 Changed 3 years ago by
But #25097 is not yet approved. The main aim in the present ticket is to later be able to lock the geometry folder (which means adding it to the list of python3knownpassing folders), so that no py3 failure could happen by accident.
comment:9 followup: 10 Changed 3 years ago by
I think this can be merged and geometry whitelisted, and then #25097 can replace the test here when that ticket is ready.
comment:10 Changed 3 years ago by
Replying to jhpalmieri:
I think this can be merged and geometry whitelisted, and then #25097 can replace the test here when that ticket is ready.
All right, sounds good!
I put the present ticket as dependancy in #25097. The ticket is essentially ready and tested, it only gets slowed due to similar conflicts/dependancies, ;) that was the reason of my reserve.
comment:11 Changed 3 years ago by
Branch:  u/jhpalmieri/27965 → 6fcdf0594952b6aa3234a75ef0d7137c84655583 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:12 Changed 3 years ago by
Milestone:  sage8.8 → sage8.9 

Not in Sage 8.8. Let's please to try keep tickets' milestones related to the release in which we actually intend to include them, and in particular the release in which they were actually included, especially when closing tickets.
very trivial ticket, please review swiftly
New commits:
py3: fix the last doctest in geometry