Opened 10 months ago

Closed 7 months ago

#27974 closed enhancement (fixed)

Implement facets method for Polyhedron

Reported by: jipilab Owned by:
Priority: major Milestone: sage-8.9
Component: geometry Keywords: polytopes, facets, days100
Cc: jipilab, gh-LaisRast, gh-kliem, selia Merged in:
Authors: Sophia Elia Reviewers: Jean-Philippe Labbé, Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: 743c4c7 (Commits) 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(dim-1)
(A 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices,
 A 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices,
 A 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices,
 A 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices,
 A 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices,
 A 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices,
 A 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices,
 A 3-dimensional 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 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices,
 A 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices,
 A 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices,
 A 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices,
 A 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices,
 A 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices,
 A 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices,
 A 3-dimensional face of a Polyhedron in ZZ^4 defined as the convex hull of 8 vertices)

Change History (22)

comment:1 Changed 10 months ago by embray

  • Milestone sage-8.8 deleted

As the Sage-8.8 release milestone is pending, we should delete the sage-8.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 (sage-8.9).

comment:2 Changed 8 months ago by selia

  • Cc selia added

comment:3 Changed 8 months ago by selia

  • Authors set to Sophia Elia
  • Branch set to public/27974
  • Commit set to 8dc7dd5cae3d0795aa75089b503fee3c5bb793a2
  • Keywords days100 added
  • Status changed from new to needs_review

New commits:

8dc7dd5implement facets

comment:4 Changed 8 months ago by chapoton

Suggestion: add cross-references, in the new method:

        .. SEEALSO:: :meth:`faces`

and the same in the other direction

comment:5 Changed 8 months ago by jipilab

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 well-tested.

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 8 months ago by git

  • Commit changed from 8dc7dd5cae3d0795aa75089b503fee3c5bb793a2 to 76a87d786f76dba3c13f7577f0bc640dd904b27e

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

76a87d7added cross-references, changed faces examples to facets where appropriate

comment:7 Changed 8 months ago by jipilab

  • Milestone set to sage-8.9
  • Reviewers set to Jean-Philippe Labbé
  • Status changed from needs_review to positive_review

LGTM, pyflakes errors are taken care of in #27993.

comment:8 Changed 8 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:9 Changed 8 months ago by jipilab

Please remove the insertion of spaces in the banner.

The conflict is probably from the library somewhere...

comment:10 Changed 7 months ago by chapoton

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 7 months ago by git

  • Commit changed from 76a87d786f76dba3c13f7577f0bc640dd904b27e to e03f2ee8b615fe80457cb4de5446f403ff744e20

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

e03f2eefixed seealso typo

comment:12 Changed 7 months ago by selia

  • Status changed from needs_work to positive_review

comment:13 Changed 7 months ago by vbraun

Merge conflict, please merge in the next beta

comment:14 Changed 7 months ago by vbraun

  • Status changed from positive_review to needs_work

comment:15 Changed 7 months ago by git

  • Commit changed from e03f2ee8b615fe80457cb4de5446f403ff744e20 to e3621b85c279f96ebe687d5ae3143c95d6dca47c

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

3362dc1implement facets
1ab0c3frebased facets on sage8.9.beta9
e3621b8fixed seealso typo

comment:16 Changed 7 months ago by git

  • Commit changed from e3621b85c279f96ebe687d5ae3143c95d6dca47c to e3177beb02a8e6269a12ae1ebfbad5ee99ade1b3

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

e3177bebanner fix

comment:17 Changed 7 months ago by git

  • Commit changed from e3177beb02a8e6269a12ae1ebfbad5ee99ade1b3 to f72960d6f0427914a4f2daa185043ca9f5a1c8ae

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

f72960dbanner fix again

comment:18 Changed 7 months ago by git

  • Commit changed from f72960d6f0427914a4f2daa185043ca9f5a1c8ae to 743c4c75f98ef8231c5c5cb82f9ff05762a85745

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

743c4c7added facets method to quickref

comment:19 Changed 7 months ago by selia

  • Status changed from needs_work to needs_review

comment:20 Changed 7 months ago by chapoton

  • Reviewers changed from Jean-Philippe Labbé to Jean-Philippe Labbé, Frédéric chapoton
  • Status changed from needs_review to positive_review

ok, good to go

comment:21 Changed 7 months ago by jipilab

  • Reviewers changed from Jean-Philippe Labbé, Frédéric chapoton to Jean-Philippe Labbé, Frédéric Chapoton

comment:22 Changed 7 months ago by vbraun

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