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

The attribute _vector of H and V representation is exposed #27709

Closed
jplab opened this issue Apr 21, 2019 · 14 comments
Closed

The attribute _vector of H and V representation is exposed #27709

jplab opened this issue Apr 21, 2019 · 14 comments

Comments

@jplab
Copy link

jplab commented Apr 21, 2019

Following the sage devel thread:

https://groups.google.com/forum/#!topic/sage-devel/iHvm9wfvr5g

The following code changes the internal attributes of V and H representations:

sage: C = polytopes.cube()
sage: C.vertices()[0].vector()[0] = 3
sage: C.vertices()[0]
A vertex at (3, -1, -1)

and:

sage: C.inequalities()[0].A()[2] = 5
sage: C.inequalities()[0]
An inequality (0, 0, 5) x + 1 >= 0

As mentioned in the __hash__ function of the file representation.py inside of the folder src/sage/geometry/polyhedron/, the attribute _vector is not immutable, due to the way it is constructed and its requirement to change in the current setup.

This ticket provides a small fix that creates a copy of the vectors.

CC: @videlec @jhpalmieri

Component: geometry

Keywords: polytopes

Author: Jean-Philippe Labbé

Branch/Commit: d969c02

Reviewer: Travis Scrimshaw

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

@jplab jplab added this to the sage-8.8 milestone Apr 21, 2019
@jplab
Copy link
Author

jplab commented Apr 21, 2019

Commit: d969c02

@jplab
Copy link
Author

jplab commented Apr 21, 2019

Branch: u/jipilab/repr_exposed

@jplab
Copy link
Author

jplab commented Apr 21, 2019

New commits:

d969c02Fixed the exposed _vector

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 21, 2019

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

8768f2eRemoved old deprecation warnings

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 21, 2019

Changed commit from d969c02 to 8768f2e

@videlec
Copy link
Contributor

videlec commented Apr 21, 2019

comment:3

Please remove the deprecation in a distinct ticket.

@tscrim
Copy link
Collaborator

tscrim commented Apr 23, 2019

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Apr 23, 2019

comment:4

LGTM.

@vbraun
Copy link
Member

vbraun commented Apr 24, 2019

comment:5

See patchbot

@tscrim
Copy link
Collaborator

tscrim commented Apr 25, 2019

comment:6

Whoops, we both missed a line.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 25, 2019

Changed commit from 8768f2e to d969c02

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 25, 2019

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

@jplab
Copy link
Author

jplab commented Apr 25, 2019

comment:8

As suggested, I removed the deprecation warning removal from the branch.

The failing doctest is related to that.

@vbraun
Copy link
Member

vbraun commented Apr 29, 2019

Changed branch from u/jipilab/repr_exposed to d969c02

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

4 participants