#27965 closed enhancement (fixed)
py3: fix the last doctest in geometry
Reported by:  chapoton  Owned by:  

Priority:  major  Milestone:  sage8.9 
Component:  python3  Keywords:  
Cc:  jipilab, vdelecroix, tscrim  Merged in:  
Authors:  Frédéric Chapoton, John Palmieri  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  6fcdf05 (Commits)  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 months ago by
 Branch set to u/chapoton/27965
 Cc tscrim added
 Commit set to 7c9a636fa9198ccf3af036ec59ba459db5664416
 Status changed from new to needs_review
comment:2 Changed 3 months 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:

src/sage/geometry/polyhedron/library.py
diff git a/src/sage/geometry/polyhedron/library.py b/src/sage/geometry/polyhedron/library.py index 71914d635a..c9bb1f35e8 100644
a b class Polytopes(): 1489 1489 Unfortunately, no polyhedra backend supports the construction of the 1490 1490 snub dodecahedron at the moment:: 1491 1491 1492 sage: sd = polytopes.snub_dodecahedron() 1492 sage: sd = polytopes.snub_dodecahedron() # py2 1493 sage: sd = polytopes.snub_dodecahedron() # py3 1494 doctest:warning 1495 ... 1496 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. 1493 1497 sage: sd.f_vector() # not tested 1494 1498 (1, 60, 150, 92, 1) 1495 1499 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 months ago by
 Branch changed from u/chapoton/27965 to u/jhpalmieri/27965
comment:4 Changed 3 months ago by
 Commit changed from 7c9a636fa9198ccf3af036ec59ba459db5664416 to 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 months ago by
 Reviewers set to Frédéric Chapoton
 Status changed from needs_review to positive_review
ok, good for me.
comment:6 Changed 3 months 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 months 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 months 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 months 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 in reply to: ↑ 9 Changed 3 months 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 months ago by
 Branch changed from u/jhpalmieri/27965 to 6fcdf0594952b6aa3234a75ef0d7137c84655583
 Resolution set to fixed
 Status changed from positive_review to closed
comment:12 Changed 3 months ago by
 Milestone changed from sage8.8 to 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