#18013: Implement abstract tableau refactoring
-------------------------------------+-------------------------------------
Reporter: jpswanson | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.6
Component: combinatorics | Resolution:
Keywords: days64, sage- | Merged in:
combinat, tableaux, refactoring, | Reviewers:
days64.25, days68 | Work issues:
Authors: Josh Swanson | Commit:
Report Upstream: N/A | fff8dbfe4e579b801ee5f9a2faabec40a144242b
Branch: public/18013 | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Comment (by jpswanson):
Thanks for the feedback Travis!
* About `__classcall_private__`: I had asked Andrew Ohana for his opinion
about this at Sage Days, and he strongly agreed with removing it; Darij
also expressed happiness that I had removed it in an email; and I
personally think it obfuscates the logic without any real benefit. So, we
seem to fundamentally disagree here--not sure what else to say. Perhaps
you could elaborate on why "we want to tightly couple that code with that
class", and how you think my current layout does not achieve that goal.
The `__classcall_private__` idiom caused quite a bit of confusion at
days64 when we first started this project, and it also fosters confusion
over where to put validation and normalization code--in an element's
`__classcall_private__`, `__init__`, or maybe somewhere on the parent?
* About elements knowing their parents: I meant they need to know they
have a parent (i.e. `el.parent()`), but they don't need to know what their
parent is. More precisely, `__classcall_private__` is the only place the
name of the parent is required in the element classes, which strikes me as
a red flag. To be clear, I'm far more interested in practical issues like
those above than in philosophical musings about OOP.
* About multiple files: having worked with this stuff a fair amount,
breaking up elements and parents is very useful if only because it's much
easier to find things. Also, there's lots of precedent for this in Sage:
Parent and Element themselves are in two files; `rational_field.py` and
`rational.pyx` are broken up, as are many other rings. About the options
file: I again disagree; it's awkward to have those options at the top of
the file since they serve a fundamentally different purpose compared to
the rest of the file. Doesn't seem like a separate file is really hurting
anything.
* About `__iter__` and `self.element_class(...)` instead of `self(...)`:
I agree optimization needs to be done, but the change you suggested won't
typically work since input normalization is done by the parent class, and
skipping it leads to subtle bugs. Figuring out the right optimizations is
one of the main things that still needs to be done IMO. I totally agree
that the current method is quite bloated.
* About `_coerce_map_from_`: hah, I dislike it too, but what you
suggested won't work (it's what I initially did). The trouble is that
hardcoding the class name (`SkewTableau` for me) causes *all* child
classes to have coercions between each other. For instance,
`SemistandardSkewTableau` would have a coercion to `StandardSkewTableau`
since they're both `SkewTableau`. The horrible hack at the start was what
Andrew came up with to get around the apparent inability to get at the
non-dynamically-generated class. This seems like an issue with the
category framework to me, but maybe there's a good solution.
* About `AbstractTableau.__init__`: ah whoops, thanks, I've at least
changed it to `super(AbstractTableau, self).__init__(*args, **kwds)`. I'd
like to just delete the method, except for the comment up top explaining
that normalization and validation should be done in the parent class.
Given the amount of confusion over where to do that in the existing
codebase, it seems worthwhile, but I'm on the fence--opinions?
* About `six`: I don't know what you mean. The use of `six` here is very
slightly different from the 2.x syntax and is only included with an eye
towards 2->3 conversion. For instance, `dict.iteritems` doesn't exist in
Python 3.
Things I'll do now: add doctests to `_element_constructor_`'s in
`ribbon_shaped_tableau.py`, add deprecations to move the functions I
mentioned, fix up the `_element_constructor_` documentation, move `_dct`
from `BadShapeTableau` to `AbstractTableau`, remove the silly
`_set_parent` call, and remove `list_method`. Those changes shouldn't take
long.
--
Ticket URL: <http://trac.sagemath.org/ticket/18013#comment:20>
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.
