#14772: Remove CombinatorialClass from Permutations
------------------------------------+------------------------------------
Reporter: tscrim | Owner: sage-combinat
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-5.11
Component: combinatorics | Resolution:
Keywords: days49 | Merged in:
Authors: Travis Scrimshaw | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Dependencies: #8386 #14519 #14808
Stopgaps: |
------------------------------------+------------------------------------
Comment (by darij):
I've done the part of the review I think I was competent enough for. Not
covered are:
-- I hope the {{{_classcall_private__}}} methods are actually doing what
they should; while this is likely given the many doctests, I am not making
any claims about this because I don't know what the heck
{{{_classcall_private__}}} means to begin with.
-- Same for {{{__setstate__}}}.
-- {{{sage/combinat/set_partition_ordered.py}}}: I'd like you to remove
that change because that's done better in #14883. (You could also review
that patch, along with #14884; they're very short.)
-- A docstring says that {{{__call__}}} is a close variant of
{{{__call__}}}:
{{{
def __call__(self, x):
"""
A close variant of ``__call__`` which just attempts to extend
the permutation.
}}}
This used to be:
{{{
def __call__(self, x):
"""
A close variant of CombinatorialClass.__call__ which just
attempts to extend the permutation
}}}
Since you probably understand this better than I do, it's best if you fix
this docstring.
-- Speed might be worth testing after I added those {{{__contains__}}}es.
Altogether, great job. If you're fine with my review patch and handle th
above four bullet points, feel free to set this to positive review!
--
Ticket URL: <http://trac.sagemath.org/ticket/14772#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/groups/opt_out.
