Opened 2 years ago
Closed 2 years ago
#27974 closed enhancement (fixed)
Implement facets method for Polyhedron
Reported by:  jipilab  Owned by:  

Priority:  major  Milestone:  sage8.9 
Component:  geometry  Keywords:  polytopes, facets, days100 
Cc:  jipilab, ghLaisRast, ghkliem, selia  Merged in:  
Authors:  Sophia Elia  Reviewers:  JeanPhilippe Labbé, Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  743c4c7 (Commits, GitHub, GitLab)  Commit:  743c4c75f98ef8231c5c5cb82f9ff05762a85745 
Dependencies:  Stopgaps: 
Description
It is often practical to get directly the facets of a polyhedron object without having to compute or to know the dimension of the object.
This ticket implements the following shortcut:
sage: c = polytopes.hypercube(4) sage: dim = c.dimension() sage: c.faces(dim1) (A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices, A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices, A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices, A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices, A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices, A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices, A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices, A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices)
Now becomes:
sage: c = polytopes.hypercube(4) sage: c.facets() (A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices, A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices, A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices, A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices, A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices, A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices, A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices, A 3dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices)
Change History (22)
comment:1 Changed 2 years ago by
 Milestone sage8.8 deleted
comment:2 Changed 2 years ago by
 Cc selia added
comment:3 Changed 2 years ago by
 Branch set to public/27974
 Commit set to 8dc7dd5cae3d0795aa75089b503fee3c5bb793a2
 Keywords days100 added
 Status changed from new to needs_review
New commits:
8dc7dd5  implement facets

comment:4 Changed 2 years ago by
Suggestion: add crossreferences, in the new method:
.. SEEALSO:: :meth:`faces`
and the same in the other direction
comment:5 Changed 2 years ago by
Suggestion: in the files base.py
, face.py
, and library.py
, perhaps it would be nice to change the tests involving faces(a_number_here)
where the intented thing is really .facets
.
This way, the function is advertized in the documentation of functions, and also welltested.
There are sufficient tests with faces
that are not facets not to have this a regression. Further, it still uses faces
anyways. So no loss in strength of testing...
One comment about the bot result: The pyflake line is taken care of in #27993.
comment:6 Changed 2 years ago by
 Commit changed from 8dc7dd5cae3d0795aa75089b503fee3c5bb793a2 to 76a87d786f76dba3c13f7577f0bc640dd904b27e
Branch pushed to git repo; I updated commit sha1. New commits:
76a87d7  added crossreferences, changed faces examples to facets where appropriate

comment:7 Changed 2 years ago by
 Milestone set to sage8.9
 Reviewers set to JeanPhilippe Labbé
 Status changed from needs_review to positive_review
LGTM, pyflakes errors are taken care of in #27993.
comment:9 Changed 2 years ago by
Please remove the insertion of spaces in the banner.
The conflict is probably from the library somewhere...
comment:10 Changed 2 years ago by
Looks good and the patchbot is green.
One detail needs to be fixed :
There is a seealso in facets referring to facets. It should refer to faces instead.
Once done, you can set to positive review on my behalf.
comment:11 Changed 2 years ago by
 Commit changed from 76a87d786f76dba3c13f7577f0bc640dd904b27e to e03f2ee8b615fe80457cb4de5446f403ff744e20
Branch pushed to git repo; I updated commit sha1. New commits:
e03f2ee  fixed seealso typo

comment:12 Changed 2 years ago by
 Status changed from needs_work to positive_review
comment:13 Changed 2 years ago by
Merge conflict, please merge in the next beta
comment:14 Changed 2 years ago by
 Status changed from positive_review to needs_work
comment:15 Changed 2 years ago by
 Commit changed from e03f2ee8b615fe80457cb4de5446f403ff744e20 to e3621b85c279f96ebe687d5ae3143c95d6dca47c
comment:16 Changed 2 years ago by
 Commit changed from e3621b85c279f96ebe687d5ae3143c95d6dca47c to e3177beb02a8e6269a12ae1ebfbad5ee99ade1b3
Branch pushed to git repo; I updated commit sha1. New commits:
e3177be  banner fix

comment:17 Changed 2 years ago by
 Commit changed from e3177beb02a8e6269a12ae1ebfbad5ee99ade1b3 to f72960d6f0427914a4f2daa185043ca9f5a1c8ae
Branch pushed to git repo; I updated commit sha1. New commits:
f72960d  banner fix again

comment:18 Changed 2 years ago by
 Commit changed from f72960d6f0427914a4f2daa185043ca9f5a1c8ae to 743c4c75f98ef8231c5c5cb82f9ff05762a85745
Branch pushed to git repo; I updated commit sha1. New commits:
743c4c7  added facets method to quickref

comment:19 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:20 Changed 2 years ago by
 Reviewers changed from JeanPhilippe Labbé to JeanPhilippe Labbé, Frédéric chapoton
 Status changed from needs_review to positive_review
ok, good to go
comment:21 Changed 2 years ago by
 Reviewers changed from JeanPhilippe Labbé, Frédéric chapoton to JeanPhilippe Labbé, Frédéric Chapoton
comment:22 Changed 2 years ago by
 Branch changed from public/27974 to 743c4c75f98ef8231c5c5cb82f9ff05762a85745
 Resolution set to fixed
 Status changed from positive_review to closed
As the Sage8.8 release milestone is pending, we should delete the sage8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage8.9).