Skip to content
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

Polyhedron.delete -> _delete #18814

Closed
nathanncohen mannequin opened this issue Jun 29, 2015 · 10 comments
Closed

Polyhedron.delete -> _delete #18814

nathanncohen mannequin opened this issue Jun 29, 2015 · 10 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jun 29, 2015

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()>

CC: @videlec @dimpase @vbraun

Component: geometry

Author: Nathann Cohen

Branch/Commit: c502543

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/18814

@nathanncohen nathanncohen mannequin added this to the sage-6.8 milestone Jun 29, 2015
@nathanncohen nathanncohen mannequin added c: geometry labels Jun 29, 2015
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jun 29, 2015

Branch: public/18814

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jun 29, 2015

Commit: 2dd3eb6

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jun 29, 2015

New commits:

2dd3eb6trac #18814: Polyhedron.delete -> _delete

@nathanncohen nathanncohen mannequin added the s: needs review label Jun 29, 2015
@vbraun
Copy link
Member

vbraun commented Jun 29, 2015

comment:2

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.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jun 29, 2015

comment:3

What exactly do you want?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 29, 2015

Changed commit from 2dd3eb6 to c502543

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 29, 2015

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

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

@dimpase
Copy link
Member

dimpase commented Sep 2, 2015

comment:5

LGTM

@dimpase
Copy link
Member

dimpase commented Sep 2, 2015

Reviewer: Dima Pasechnik

@vbraun
Copy link
Member

vbraun commented Sep 4, 2015

Changed branch from public/18814 to c502543

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants