Opened 2 years ago

Closed 2 years ago

#28603 closed enhancement (fixed)

CombinatorialPolyhedron: edge_graph -> vertex_graph

Reported by: gh-kliem Owned by:
Priority: major Milestone: sage-9.0
Component: geometry Keywords: polytopes, combinatorial polyhedron
Cc: jipilab, gh-LaisRast Merged in:
Authors: Jonathan Kliem Reviewers: Laith Rastanawi
Report Upstream: N/A Work issues:
Branch: ecb7986 (Commits, GitHub, GitLab) Commit: ecb7986f9b97849454654fea747c6ba779f7ce5f
Dependencies: Stopgaps:

Status badges

Description (last modified by gh-kliem)

In order to make CombinatorialPolyhedron more consistent with Polyhedron, we replace edge_graph by vertex_graph.

In case of of unbounded polyhedra this might make a difference, as unbounded 1-faces are considered for edge_graph but not for vertex_graph.

For now we keep edge_graph and add a deprecation warning.

Change History (12)

comment:1 Changed 2 years ago by gh-kliem

  • Description modified (diff)

comment:2 Changed 2 years ago by gh-kliem

  • Branch set to public/28603
  • Commit set to a005e47bdcec75d23bc592788d935524548c0723

New commits:

a005e47added vertex_graph, deprecated edge_graph

comment:3 Changed 2 years ago by gh-kliem

  • Cc jipilab gh-LaisRast added
  • Status changed from new to needs_review

comment:4 Changed 2 years ago by gh-LaisRast

  • Reviewers set to Laith Rastanawi
  • Status changed from needs_review to positive_review

It is good to go.

comment:5 Changed 2 years ago by git

  • Commit changed from a005e47bdcec75d23bc592788d935524548c0723 to 96346fa171ef754e17e4daeca456774b41b2a6db
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

96346faimproved deprecation warning

comment:6 Changed 2 years ago by gh-LaisRast

  • Status changed from needs_review to positive_review

comment:7 follow-up: Changed 2 years ago by jipilab

Could the sentence also include what function the use should use instead?

comment:8 Changed 2 years ago by git

  • Commit changed from 96346fa171ef754e17e4daeca456774b41b2a6db to 4b83abee14205337f83edb6430d8c83cad7a667c
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

4b83abedeprecation warning gives new name

comment:9 in reply to: ↑ 7 Changed 2 years ago by gh-kliem

Replying to jipilab:

Could the sentence also include what function the use should use instead?

Sure.

The deprecation warning doesn't show in normal use. When you run the doctest manually it won't show. If you pack it in

sage: def foo(C):
sage:     return C.edge_graph()

it is printed when calling foo.

comment:10 Changed 2 years ago by git

  • Commit changed from 4b83abee14205337f83edb6430d8c83cad7a667c to ecb7986f9b97849454654fea747c6ba779f7ce5f

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

ecb7986changed stacklevel to 3, so deprecation warning shows during normal usage

comment:11 Changed 2 years ago by gh-LaisRast

  • Status changed from needs_review to positive_review

The deprecation warning is now printed out. So I will put it back on "positive review".

comment:12 Changed 2 years ago by vbraun

  • Branch changed from public/28603 to ecb7986f9b97849454654fea747c6ba779f7ce5f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.