#18564: Boost Edge Connectivity
-------------------------------------+-------------------------------------
Reporter: borassi | Owner: borassi
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-6.8
Component: graph theory | Resolution:
Keywords: Boost, | Merged in:
connectivity | Reviewers: Nathann Cohen
Authors: Michele Borassi | Work issues:
Report Upstream: N/A | Commit:
Branch: | 51a183a065af09d203be1f01857b4045a1afee4d
u/borassi/boost_edge_connectivity | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Changes (by ncohen):
* reviewer: => Nathann Cohen
Comment:
Helloooooo Michele,
I made several modifications, available as a new commit at `public/18564`.
All
of them are, of course, open to discussion.
- You don't need two 'pass' in the .pxd file
- About the paragraphs at the top of `boost_graph.pyx`: I find them a bit
unclear. Could you be more specific in those sentences? For example (but
not
only), I do not get the meaning of
{{{
and they cannot be shared by different Python functions, because they
cannot
be converted to Python objects.
}}}
- I opened a ticket at #18753 (you are its author) to indicate that the
boost
bug had been reported upstream. That's part of a 'procedure' called
'stopgap',
precisely meant for those situations (see
http://doc.sagemath.org/html/en/developer/trac.html#stopgaps)
I also turned your 'print warning' into a stopgap message.
- I reversed the names of `edge_connectivity` and
`boost_edge_connectivity`: now
the `boost_edge_connectivity` function is the one which takes a boost
graph as
argument.
- The link toward `edge_connectivity` that you had added at the top of the
document was broken, for `edge_connectivity` was a cdef function in your
patch, and you cannot link (in Sphinx) toward cdef functions. When yo
uwork on
the doc, you can compile it with `--warn-links` to spot broken links.
- I shortened a bit the doc of some boost functions: some of the things
you say
in the doc would be more appropriate as comments in the code.
- in simple cases, it is easier to use `sig_check` than `sig_on/off`. A
`sig_check` is a way to check at a specific time whether an exception
should
be raised. See
http://www.sagemath.org/documentation/html/en/developer/coding_in_cython.html
#interrupt-and-signal-handling
- I renamed 'boost=True' to 'implementation="boost"', which is a bit more
'standard'. You may not know what 'boost' is, but you understand the
meaning
of a 'implementation' selector.
Again, don't hesitate to oppose and discuss any of the changes in this
commit,
as you are the reviewer of them. You are supposed to be as picky as we are
when
we review your code.
In my mind, the branch with this commit added is good to go, though
perhaps the
comments to the top of `boost_graph.pyx` could be made clearer. That's up
to
you.
Have fun, and thank you very much again for this addition. Now that this
is
written, it will be *much* easier to steal more boost code `;-)`
Nathann
--
Ticket URL: <http://trac.sagemath.org/ticket/18564#comment:48>
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 http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
