Opened 5 years ago

Closed 5 years ago

#18814 closed defect (fixed)

Polyhedron.delete -> _delete

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.9
Component: geometry Keywords:
Cc: vdelecroix, dimpase, vbraun Merged in:
Authors: Nathann Cohen Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: c502543 (Commits) Commit: c502543abf33d3e6c34d6ed98e46fb76d1a08d9b
Dependencies: Stopgaps:

Description

From the doc of Polyhedron.delete (which is a immutable/hashable object): 

        Delete this polyhedron.

        This speeds up creation of new polyhedra by reusing
        objects. After recycling a polyhedron object, it is not in a
        consistent state any more and neither the polyhedron nor its
        H/V-representation objects may be used any more.

This really shouldn't be exposed at user level: 

sage: p = polytopes.cube()
sage: p.delete()
sage: p
/home/ncohen/.Sage/local/lib/python2.7/site-packages/sage/repl/rich_output/display_manager.py:570: RichReprWarning: Exception in _rich_repr_ while displaying object: object of type 'NoneType' has no len()
  RichReprWarning,
<repr(<sage.geometry.polyhedron.backend_ppl.Polyhedra_ZZ_ppl_with_category.element_class at 0x7f1f17c92c30>) failed: TypeError: object of type 'NoneType' has no len()>

Change History (6)

comment:1 Changed 5 years ago by ncohen

  • Branch set to public/18814
  • Commit set to 2dd3eb6772d0955170def9f3ef2294be12935bce
  • Status changed from new to needs_review

New commits:

​2dd3eb6trac #18814: Polyhedron.delete -> _delete

comment:2 follow-up: Changed 5 years ago by vbraun

Renaming it doesn't really address your point that it is immutable.

I also thought we are all consenting adults.

I agree that a better implementation should be provided, but just renaming it is not doing anything.

comment:3 in reply to: ↑ 2 Changed 5 years ago by ncohen

What exactly do you want?

comment:4 Changed 5 years ago by git

  • Commit changed from 2dd3eb6772d0955170def9f3ef2294be12935bce to c502543abf33d3e6c34d6ed98e46fb76d1a08d9b

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

​5bcf79fMerge branch 'public/18814' into 6.9.b4
​c502543trac #18814 fixing doctest continuation

comment:5 Changed 5 years ago by dimpase

  • Milestone changed from sage-6.8 to sage-6.9
  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

LGTM

comment:6 Changed 5 years ago by vbraun

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