#7477: Matroids
----------------------------------------------------+-----------------------
Reporter: ncohen | Owner: jkantor
Type: enhancement | Status:
needs_review
Priority: major | Milestone: sage-5.10
Component: combinatorics | Resolution:
Keywords: | Work issues:
Report Upstream: N/A | Reviewers:
Authors: Stefan van Zwam, Rudi Pendavingh | Merged in:
Dependencies: | Stopgaps:
----------------------------------------------------+-----------------------
Comment (by vbraun):
Looks pretty solid. I think you can review the mathematical correctness
among yourself. You should have somebody who is more familiar with
Sageisms to look at code style, then it should be ready to go. There are
some small code style issues that would have been easier if you had
started with a smaller chunk of code.
PEP8 whitespace http://www.python.org/dev/peps/pep-0008/#other-
recommendations
{{{
i = i + 1 # yes
i=i+1 # no
}}}
Docstring markup http://www.sagemath.org/doc/developer/conventions.html
#docstring-markup-with-rest-and-sphinx:
* `EXAMPLES::` not `EXAMPLE::` (though thats also misspelled in other
places in the sage library)
* Imperative: "Test that foo is bar" instead of "Tests that foo is bar".
* `INPUT:` should include type information and our formatting:
{{{
- ``n``: The dimension of the projective space. # no
- ``n`` -- positive integer. The dimension of the projective space. # yes
}}}
* We have markup for referencing other docstrings
{{{
... see :class:`OtherClass` ...
... or :meth:`other_method` ...
}}}
that you might want to use more consistently.
The !SetSystem should probably be factored out and integrated into
`sage.sets`.
Your private reimplementation of all matrix functionality has a lot of
code-smell. If the only reason is that pivoting is too slow then you
should look into fixing that instead of writing your own matrix
implementation.
Whats this (left-over debugging?):
{{{
#if d>0:
# F=Fa+Fb
#
self._q_projection=self._q_projection.matrix_from_rows_and_columns(F,F)
}}}
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/7477#comment:23>
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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
