Implement star and stellar subdivision of a face of simplicial complex

[jipilab]

This ticket provides the star and the stellar subdivision of a face of a simplicial complex.

The star of a face is the union of the faces that contains that face. The star of the empty face is the whole complex.

Given a simplicial complex C, the stellar subdivision of a face f of C is a new simplicial complex obtained as the barycentric subdivision of the face with respect to its star.

[jipilab]

New commits:

​2dfd035

First implementation of star

[git]

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

​04bcbef

Added first implementation of stellar subdivision

[git]

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

​c6b3853	Corrected indentation
​e948253	pep8 conventions

[tmonteil]

First few comments/questions:

• would it be pertinent to check that the provided simplex is actually a simplex in the considered simplicial complex ?
• You could provide non-working examples/tests to check such inconsistent user inputs.
• in stellar_subdivision, the mutable option is not taken into account (does not appear in the code).
• what is the difference between is_mutable and mutable option ?
• you should add an INPUT section in stellar_subdivision.

[git]

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

​fae5016

Review and made tests pass

[chapoton]

missing empty line here:

+        EXAMPLES::
+            sage: SC = SimplicialComplex([[0,1,2],[1,2,3]])

[git]

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

​eb4dfac

pep8 conventions

[git]

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

​9ed5260

Corrected a silly typo

[jhpalmieri]

You should avoid lines longer than 80 characters. I see one here:

- `is_mutable` -- (optional) boolean, determines if the output is mutable, default ``True``

In a line like

def stellar_subdivision(self,simplex,inplace=False,is_mutable=True):

the style is to put a space after each comma.

In docstrings, use single backquotes or dollar signs – `F`, $F$ – for math, but use double backquotes – ``simplex``, ``is_mutable`` – for code.

In doctests, you need to put a double colon :: before each indented line, not just a single colon. For example, you need two colons at the end of this line

The simplex to subdivide should be a face of self::

I'm assuming that you haven't actually looked at the built documentation or you would have realized that there were problems. It's good practice to look at the html documentation to make sure it is okay.

[git]

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

​2c1c6a8

pep8 conventions and fixed doc

[jipilab]

Dear jhpalmieri,

Thank you for the comments. Indeed, the double ticks slipped out of my mind for the code part.

I fixed the documentation, hopefully it should be okay now.

[jhpalmieri]

Okay, better, but now the indentation doesn't match in the input and output blocks. You need to make changes like

src/sage/homology/simplicial_complex.py

@@ -2776 +2776 @@
 
         - ``simplex`` -- a simplex in this simplicial complex
         - ``is_mutable`` -- (default: ``True``) boolean; determines if the output
-            is mutable
+          is mutable
 
         EXAMPLES::
 

[git]

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

​d68a1a2

Corrected indentation of input blocks

[jipilab]

Oh! Okay, my bad! Should be okay now.

Thanks a lot for the help.

[chapoton]

1) The first line of the 2 new methods should rather be

+        """
+        Return the star of a simplex in this simplicial complex.

and

+        """
+        Return the stellar subdivision of a simplex in this simplicial complex.

2) in the raise statements the convention is not to start with a capital and not to end with a dot, so something like

+            raise ValueError("this simplicial complex is not mutable")

and

+            raise ValueError("the face to subdivide is not a face of self")

and then change the doctests accordingly.

3) I would add "or on a copy" here:

+        - ``inplace`` -- (default: ``False``) boolean; determines if the
+          operation is done on ``self`` or on a copy

Otherwise, this looks good.

[git]

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

​1b54fba

corrected conventions

[chapoton]

ok, let it be