Add neighborly methods for polyhedra

[mo271]

I propose to add two functions to the polyhedron class; namely neighborliness and is_neighborly.

While neighborliness could be used to calculate the is_neighborly, it might be faster not to compute the neighborliness if one only wants to know is_neighborly(k) for some small k. (More specifically, the output of P.is_neighborly() shoud be the same as P.neighborliness()>=floor(P.dim()/2))).

CC: @jplab @fchapoton @videlec

Component: geometry

Keywords: polyhedron, days84

Author: Moritz Firsching

Branch/Commit: eea1215

Reviewer: Jean-Philippe Labbé

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

[mo271]

Branch: u/moritz/neighborly

[sagetrac-git]

Commit: 941c9bd

[sagetrac-git]

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

941c9bd

undo accidental unwanted modification of truncation method

[jplab]

comment:3

Hi Moritz,

You could truncate the two long lines before the "for" (see https://www.python.org/dev/peps/pep-0008/#maximum-line-length )

[jplab]

comment:4

and you can put your name as the author.

[mo271]

Author: Moritz Firsching

[mo271]

Changed keywords from polyhedron to polyhedron days84

[sagetrac-git]

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

e147192

improved docstring

[sagetrac-git]

Changed commit from 941c9bd to e147192

[videlec]

comment:8

k = 1
while(True):
    if len(self.faces(k))==binomial(self.n_vertices(),k+1):
        k += 1
    else:
        return k

can be simplified to

k = 1
while len(self.faces(k)) == binomial(self.n_vertices(), k + 1):
    k += 1
return k

(please be careful with spaces in the code)

[videlec]

comment:9

About spacing, change (in the code and in the examples)

• a=b to a = b, a>b to a > b, etc
• f(x,y) to f(x, y)

[sagetrac-git]

Changed commit from e147192 to 8552363

[sagetrac-git]

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

17e931e	added 'neighborliness' and 'is_neighborly' methods
a05912e	undo accidental unwanted modification of truncation method
07e8c58	improved docstring
8552363	spaces and more doctests

[mo271]

comment:11

Thanks Vincent!
I modified the spaces and added more doctests.

---

New commits:

17e931e	added 'neighborliness' and 'is_neighborly' methods
a05912e	undo accidental unwanted modification of truncation method
07e8c58	improved docstring
8552363	spaces and more doctests

[jplab]

comment:12

Hi Moritz,

• perhaps in the "neighborliness" add a SEEALSO pointing to "is_neighborly".
• In the examples of neighborliness there are spaces missing in P=Polyhedron in several instances.
• In the INPUT and OUTPUT of is_neighborly there are backticks missing around one k.

More comments to come...

[sagetrac-git]

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

be71a2e

added SEEALSO, links to wikipedia and improved docstrings

[sagetrac-git]

Changed commit from 8552363 to be71a2e

[jplab]

Changed keywords from polyhedron days84 to polyhedron, days84

[jplab]

Reviewer: Jean-Philippe Labbé

[jplab]

comment:15

This ticket looks ready to go to me.

I'd like to have a second opinion though to make sure!

[jplab]

comment:16

Hi Moritz,

Here are a couple of things to correct:

• the syntax for the seealso block (in both functions) should be corrected (see here)
• The seealso block should be before the examples
• The first example in is_neighborly should be easy. Put the first example at the end.
• perhaps change the indexl so that it is not confused with the number 1.

The doc compiles on 7.6.beta6

[sagetrac-git]

Changed commit from be71a2e to c994323

[sagetrac-git]

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

c994323

improved docstring

[mo271]

comment:18

Thanks JP! I improved the docstring.

[jplab]

comment:20

Looks good! Up to the bot, I'll give it a positive review.

[sagetrac-git]

Changed commit from c994323 to 604b46b

[sagetrac-git]

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

604b46b

fixed two pep8 errors

[jplab]

comment:22

I would adapt the text before the neighborliness examples (specifying the cubes).

All test pass on 7.6.beta6 and the documentation looks good.

Once the bot gives the green light and you corrected the above, I'd give it a positive review.

[sagetrac-git]

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

eea1215

moved comment about cyclic polytopes

[sagetrac-git]

Changed commit from 604b46b to eea1215

[mo271]

comment:24

thanks, JP!

I moved the text before the cubes to the cyclic example.

[jplab]

comment:25

It now looks good to me. Positive review.

[vbraun]

Changed branch from u/moritz/neighborly to eea1215