New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Placing triangulation and normal cones #12159
Comments
Initial patch |
Attachment: trac_12159_separate_triangulation_file.patch.gz Initial patch |
This comment has been minimized.
This comment has been minimized.
comment:1
Attachment: trac_12159_placing_triangulation.patch.gz |
This comment has been minimized.
This comment has been minimized.
comment:3
A bunch of doctests fail after installing the TOPCOM package, presumably just because of different choices of triangulations. I can't think of a nice way to deal with that offhand, it would be pretty clunky to force every example to use the Sage-native code. |
comment:4
OK, maybe its not so bad. Adding in a few more "set_engine('internal')" and "engine='internal'" lines to element.py in geometry/triangulation is enough to fix all the doctest errors. |
comment:5
I imagine that is trivial for Volker to do himself but in case its helpful here is my version that passes the doctests after TOPCOM is installed: |
Attachment: trac_12159_normal_cone.2.patch.gz Updated patch |
Updated patch |
comment:6
Attachment: trac_12159_normal_cone.patch.gz Updated patch fixes doctests with TOPCOM installed. |
comment:7
Apply trac_12159_separate_triangulation_file.patch, trac_12159_placing_triangulation.patch, trac_12159_normal_cone.patch (for the patchbot) |
comment:8
I have not checked the mathematics of this patch in depth, but it passes all doctests and coverage checks, the documentation looks good, and what I have checked seems correct. The problems with doctests after TOPCOM is installed have been fixed. So I am happy to give it a positive review. |
Reviewer: Marshall Hampton |
Merged: sage-5.0.beta9 |
This ticket implements some more functionality for triangulations:
Apply
CC: @novoselt
Component: geometry
Author: Volker Braun
Reviewer: Marshall Hampton
Merged: sage-5.0.beta9
Issue created by migration from https://trac.sagemath.org/ticket/12159
The text was updated successfully, but these errors were encountered: