Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#16625 closed defect (fixed)

Better plotting for polytopes, re-add projection_direction

Reported by: Karl-Dieter Crisman Owned by:
Priority: major Milestone: sage-6.3
Component: geometry Keywords:
Cc: Dima Pasechnik, Volker Braun, Andrey Novoseltsev Merged in:
Authors: Volker Braun Reviewers: Andrey Novoseltsev
Report Upstream: N/A Work issues:
Branch: bdb1803 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Karl-Dieter Crisman)

From this ask.sagemath question, the two following should be different, but aren't.

poly = polytopes.twenty_four_cell()
poly.show()

poly.show(projection_direction=[2,5,11,17])

Change History (16)

comment:1 Changed 8 years ago by Karl-Dieter Crisman

Description: modified (diff)

comment:2 Changed 8 years ago by Karl-Dieter Crisman

Even P8prime = P8.base_extend(QQ) doesn't seem to have the schlegel method, which is even more perplexing.

Last edited 8 years ago by Karl-Dieter Crisman (previous) (diff)

comment:3 Changed 8 years ago by Karl-Dieter Crisman

Description: modified (diff)
Summary: Schlegel doesn't work for all polytopesprojection_direction broken for polytopes

comment:4 Changed 8 years ago by Volker Braun

Polyhedra have a schlegel_projection method, and not a schlegel method. The internally used Projection class has a schlegel method.

comment:5 in reply to:  4 Changed 8 years ago by Karl-Dieter Crisman

Polyhedra have a schlegel_projection method, and not a schlegel method. The internally used Projection class has a schlegel method.

Yes, there was some confusion in my mind earlier...

As to your comment on the ask.sagemath question, unfortunately the following is still in the documentation.

sage: poly = polytopes.twenty_four_cell()
sage: poly
A 4-dimensional polyhedron in QQ^4 defined as the convex hull of 24 vertices
sage: poly.show()
sage: poly.show(projection_direction=[2,5,11,17])

So if that is not supposed to be used in that way, it was missed whenever those methods changed. There is also no deprecation information given.

comment:6 Changed 8 years ago by Volker Braun

Branch: u/vbraun/projection_direction_broken_for_polytopes

comment:7 Changed 8 years ago by Volker Braun

Authors: Volker Braun
Commit: fa898d1c2761a9b0cb180d890eb490034daad09b
Status: newneeds_review
Summary: projection_direction broken for polytopesBetter plotting for polytopes, re-add projection_direction

New commits:

fa898d1Better plotting of polyhedra

comment:8 Changed 8 years ago by Karl-Dieter Crisman

Wow, that is a lot more than need to fix this :-) It's gone beyond what I can comfortably review in a few minutes, but overall the structure, deprecations, and doc look like a big improvement.

comment:9 Changed 8 years ago by git

Commit: fa898d1c2761a9b0cb180d890eb490034daad09b8a213a725a158e44ffb12bb1dffd5b6bf43f6514

Branch pushed to git repo; I updated commit sha1. New commits:

8a213a7Make projection objects display as graphics

comment:10 Changed 8 years ago by git

Commit: 8a213a725a158e44ffb12bb1dffd5b6bf43f6514cf41e5b4608202dfe4fca529322f73c64738d0b5

Branch pushed to git repo; I updated commit sha1. New commits:

cf41e5bFix deprecations and doctests

comment:11 Changed 8 years ago by Volker Braun

anybody feels like reviewing this?

comment:12 Changed 8 years ago by Andrey Novoseltsev

Reviewers: Andrey Novoseltsev

Working on it.

comment:13 Changed 8 years ago by Andrey Novoseltsev

Branch: u/vbraun/projection_direction_broken_for_polytopesu/novoselt/projection_direction_broken_for_polytopes

comment:14 Changed 8 years ago by Andrey Novoseltsev

Commit: cf41e5b4608202dfe4fca529322f73c64738d0b5bdb1803badb326172dd310a79033ecbdb49904b6
Status: needs_reviewpositive_review

lgtm and works fine!


New commits:

bdb1803Reorder arguments description.

comment:15 Changed 8 years ago by Volker Braun

Branch: u/novoselt/projection_direction_broken_for_polytopesbdb1803badb326172dd310a79033ecbdb49904b6
Resolution: fixed
Status: positive_reviewclosed

comment:16 Changed 8 years ago by Karl-Dieter Crisman

Commit: bdb1803badb326172dd310a79033ecbdb49904b6

Just FYI, there was no doctesting of

+    def show(self, *args, **kwds):
+        from sage.misc.superseded import deprecation
+        deprecation(16625, 'use Projection.plot instead')
+        return self.plot(*args, **kwds)

followup will be part of #17247.

Note: See TracTickets for help on using tickets.