-
-
Notifications
You must be signed in to change notification settings - Fork 480
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
Improve the output of repr_pretty_Hrepresentation for Polyhedron #24837
Comments
Dependencies: #22572 |
Commit: |
Branch: u/jipilab/24837 |
Author: Jean-Philippe Labbé |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Branch pushed to git repo; I updated commit sha1. New commits:
|
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
|
Changed keywords from days93 to days93, IMA-PolyGeom |
Changed branch from u/jipilab/24837 to u/moritz/24837 |
Reviewer: Moritz Firsching |
comment:8
There was one doctest failing and a bit whitespace left. Other than that I think this is good to go. Set it on positive review on my behalf if you agree! New commits:
|
comment:9
The last bot test looks good. I'm setting it to positive review. |
comment:11
Merge conflict |
comment:13
Ping! |
Changed reviewer from Moritz Firsching to Moritz Firsching, Travis Scrimshaw |
comment:14
Two trivial failures; see patchbot for details:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:16
Should now work... |
comment:17
There are two pyflakes warnings about the usage of Question: if they are used in doctests, that's not necessary to import them, correct? If so, I'd just remove them... |
comment:18
Replying to @jplab:
Please do. What is imported in a file and what used in doctests, which is the global namespace, are essentially independent. Once you remove them, you can set a positive review on my behalf. |
comment:19
Replying to @tscrim:
Sounds good! I just wanted to make sure... |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:21
Done! Thanks for the review! |
comment:22
update milestone 8.3 -> 8.4 |
Changed branch from u/jipilab/24837 to |
Changed commit from |
comment:24
There is a follow-up ticket #26141 concerning the code quality. |
Following #22572, we can improve the output of the pretty print:
In the new version it gives:
The
style
parameter allows to change the way to print the H-relations:In order to make the function more apparent, deprecation of the current function is perhaps a good idea and change the name to
Hrepresentation_str
.Depends on #22572
CC: @videlec @mo271 @tscrim
Component: geometry
Keywords: days93, IMA-PolyGeom
Author: Jean-Philippe Labbé
Branch:
b1ae583
Reviewer: Moritz Firsching, Travis Scrimshaw
Issue created by migration from https://trac.sagemath.org/ticket/24837
The text was updated successfully, but these errors were encountered: