#19936: Make num_faces
-------------------------------------+-------------------------------------
Reporter: kcrisman | Owner:
Type: enhancement | Status: needs_review
Priority: minor | Milestone: sage-7.1
Component: graph theory | Resolution:
Keywords: | Merged in:
Authors: Marco Cognetta | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/mcognetta/make_num_faces | 748faac558b4d938923991f19b65bbaa2e012ddc
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by tscrim):
Thanks for making this contribution. Here are some comments on what you
will need to do in order to get this into Sage:
- It's a good idea to not remove the raw string formatting (i.e., the `r`
before a string) unless there is a specific reason to do so. Even more so
when it is in a part unrelated to the ticket.
- Similarly, it's not a good idea to add whitespace in a place unrelated
to the ticket.
- You should try to conform to PEP8 as much as possible.
- You need to also make the one-line documentation a declarative
statement.
- You also need a blank line after `TESTS::`.
- I don't see a reason to catch the `ValueError` being thrown by `faces`.
Thus, I would make the following changes:
{{{#!diff
- def num_faces(self,embedding = None):
+ def num_faces(self, embedding=None):
"""
- Returns the number of faces of an embedded graph.
+ Return the number of faces of an embedded graph.
EXAMPLES::
sage: T = graphs.TetrahedralGraph()
sage: T.num_faces()
4
TESTS::
+
sage: G = graphs.CompleteMultipartiteGraph([3,3])
sage: G.num_faces()
Traceback (most recent call last):
...
ValueError: No embedding is provided and the graph is not
planar.
"""
- try:
- return len(self.faces(embedding))
- except ValueError as e:
- raise ValueError(e)
+ return len(self.faces(embedding))
}}}
Personally I don't like the blank lines around the `"""`, especially on
both sides, but that is much more a stylistic thing.
--
Ticket URL: <http://trac.sagemath.org/ticket/19936#comment:3>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
