Opened 17 months ago
Closed 16 months ago
#28828 closed defect (fixed)
Attributes of polyhedra are exposed
Reported by: | gh-kliem | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.0 |
Component: | geometry | Keywords: | polyhedron, exposed attributes |
Cc: | jipilab, gh-LaisRast | Merged in: | |
Authors: | Jonathan Kliem | Reviewers: | Laith Rastanawi |
Report Upstream: | N/A | Work issues: | |
Branch: | a7eeece (Commits, GitHub, GitLab) | Commit: | a7eeece2180be4d91587eb133e7ab73734400f85 |
Dependencies: | Stopgaps: |
Description (last modified by )
Currently the f_vector is exposed.
sage: P = polytopes.simplex() sage: P.f_vector()[0] = 2 sage: P.f_vector() (2, 4, 6, 4, 1)
Same applies for
- incidence matrix,
- vertex-facet graph,
- vertices matrix,
- vertex adjacency matrix,
- facet adjacency matrix,
- gale transform.
Some of the above are probably more relevant than others.
Change History (12)
comment:1 Changed 17 months ago by
- Description modified (diff)
- Keywords exposed attributes added; f_vector removed
- Summary changed from f_vector is exposed to Attributes of polyhedra are exposed
comment:2 Changed 16 months ago by
comment:3 Changed 16 months ago by
Thanks for pointing this out.
As for vectors and matrices there is set_immutable
, which should work fine here.
I just didn't get around to doing it.
comment:4 Changed 16 months ago by
- Branch set to public/28828
- Commit set to c1138025fd8a25a48882b483c8bad5399d296ec0
New commits:
c113802 | make exposed invariants of polyhedron immutable
|
comment:5 Changed 16 months ago by
- Status changed from new to needs_review
comment:6 follow-up: ↓ 10 Changed 16 months ago by
I am not sure we need to change everything to be immutable. But since you are doing this, you may also want to consider the following:
sage: P = polytopes.cube() ....: P.restricted_automorphism_group(output='matrixlist')[0][0,0] = 1000 ....: P.restricted_automorphism_group(output='matrixlist')[0] ....: [1000 0 0 0] [ 0 1 0 0] [ 0 0 1 0] [ 0 0 0 1]
comment:7 Changed 16 months ago by
- Status changed from needs_review to needs_work
comment:8 Changed 16 months ago by
- Commit changed from c1138025fd8a25a48882b483c8bad5399d296ec0 to a7eeece2180be4d91587eb133e7ab73734400f85
comment:9 Changed 16 months ago by
- Status changed from needs_work to needs_review
comment:10 in reply to: ↑ 6 Changed 16 months ago by
As mentioned in the description of the ticket, some methods are worse than others.
E.g. it can easily happen, that one does something with the graph. There is even an example in the doctests (the one I modified in ticket), where the original graph was reversed.
Replying to gh-LaisRast:
I am not sure we need to change everything to be immutable. But since you are doing this, you may also want to consider the following:
sage: P = polytopes.cube() ....: P.restricted_automorphism_group(output='matrixlist')[0][0,0] = 1000 ....: P.restricted_automorphism_group(output='matrixlist')[0] ....: [1000 0 0 0] [ 0 1 0 0] [ 0 0 1 0] [ 0 0 0 1]
comment:11 Changed 16 months ago by
- Reviewers set to Laith Rastanawi
- Status changed from needs_review to positive_review
I believe now it is good to go.
comment:12 Changed 16 months ago by
- Branch changed from public/28828 to a7eeece2180be4d91587eb133e7ab73734400f85
- Resolution set to fixed
- Status changed from positive_review to closed
An older open ticket might be worth a look in this context: https://trac.sagemath.org/ticket/25509. It proposes to simplify the creation of immutable vectors/matrices, e.g. by adding an
immutable=True/False
switch to thevector
andmatrix
constructors.