#18448: Add test if a matroid is binary
-------------------------------------+-------------------------------------
Reporter: Rudi | Owner: Rudi
Type: enhancement | Status: needs_review
Priority: minor | Milestone: sage-6.8
Component: matroid theory | Resolution:
Keywords: | Merged in:
Authors: Rudi Pendavingh | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/Rudi/add_test_if_a_matroid_is_binary|
edaf4c7793369b2395dc241ad40a7d4452d93c39
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by tscrim):
Replying to [comment:11 Rudi]:
> thanks for the feedback.
Thanks for your work.
> Why is `isinstance(self, BinaryMatroid)` preferred over over `type(self)
== BinaryMatroid`? For a class like BinaryMatroid that has no derived
classes, both seem to accomplish the exact same thing.
Because then it checks for subclasses, i.e., if X is an instance of a
subclass of `BinaryMatroid`, then `isinstance(X, BinaryMatroid)` returns
`True`, whereas `type(X) == BinaryMatroid` returns `False`. Suppose a user
has implemented a subclass in private code, then it should work for those
subclasses too (since they are suppose to behave as a `BinaryMatroid`
would).
> > - You need to fix the documentation of your methods. For example, the
`INPUT:` block should only have one colon. You can look at the other
nearby methods or ask me if you have any questions. I'd also add some
latex formating to things using single backticks, e.g., {{{`|X\B| \leq
1`}}}.
>
> I improved the docstring, with an eye on adjacent methods.
>
> I wrote dozens of docstrings just before we added the matroid monster
patch (essentialy all the docstrings were written after we wrote all the
code), but apparently the rules for proper docstring writing have escaped
me (or perhaps Stefan repaired my bad formatting during the review, so
that I never even realized all my errors). Anyway, I should learn to do it
properly. I'll take a look at the developer guide.
It's a lot better (and overall, your documentation is good), but there's
still some work to do:
- The indentation on all blocks with only a single colon should be the
same as the base:
{{{
INPUT:
- ``randomized_tests`` -- (default: 1) some description here
}}}
- You should have (with 2 colons, space, and empty line):
{{{
.. SEEALSO::
:meth:`M.is_binary() <sage.matroids.matroid.Matroid.is_binary>`
}}}
- The `EXAMPLES::` block should be followed by an empty line.
- Change `<=` to `\leq` (for the interactive doc, we make this
substitution, but it will look better in the compiled doc).
- For the `BinaryMatroid.is_binary` and for `RegularMatroid`, I would just
say the `randomized_tests` input is ignored.
--
Ticket URL: <http://trac.sagemath.org/ticket/18448#comment:12>
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.
