#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 | 37ace348fb1afdb930b0657a80e40a58aea14be2
Branch: public/18013 | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Comment (by jpswanson):
Thanks again for your thoughts!
I don't think I quite communicated my intent about "philosophical
musings". You say they're "to help ease the burden of code maintenance",
which I'd say is a perfectly reasonable "practical issue"--it's one of the
most important when writing code. I just find an argument like "X is a red
flag" or "Y should be tied closely to Z" to be generally much less
convincing than "X is hard to maintain" or "Y is hard to find if it's not
near Z". I'd call the former "philosophical musings" and the latter
"practical issues".
I'll think further on your comments about `__classcall_private__`. Ease of
code maintenance is one of the biggest reasons I removed it, but
nonetheless I'll weigh the options again. I know it's a common idiom in
combinat, though grepping for it shows that it doesn't seem to have caught
on much elsewhere in the Sage codebase. I would imagine a number of our
disagreements stem from combinat doing something one way and me not being
familiar with/not wanting to use that way, which isn't necessarily a bad
thing, especially when it's not a clear-cut case of "house style".
I'll also rethink where validation and normalization "should" or "should
be expected" to be done. I don't see much potential for "greatly
simplified code"--perhaps you could be more specific about what
simplifications you envision. If you mean that less work could be done on
element creation in specific instances like making an SSYT from an SYT,
then sure.
Again we agree that iteration optimization is essentially required. It's
probably the single most important use of the SSYT and SYT code, and
there's tons of nested, pointless resolutions, normalizations, and copies
going on currently.
At the end you rather suddenly suggested a significant refactoring, where
as far as I can tell you want `SkewTableau` to inherit from a second base
class which is built on the `_st` representation of tableaux, so
`SkewTableau` would use both `_st` and `_dct`. The specifics and benefits
are unclear to me--I'd have to reimplement many of the routines currently
on `AbstractTableau` in the new context using the `_st` representation,
whereas the whole point of this refactoring was to cut down on code
duplication. (Or, perhaps you wanted the `_st` ABC to be very
minimalistic? If so, what's the point? Do you expect both ABC's to have
`to_word`, for instance, and if so why?) I tend to think of the `_st`
representation used in `_dct` as an optimization since it's how existing
routines and classes want to deal with or generate these tableaux, rather
than as a fundamentally different design choice, which makes me much more
okay with the current abstraction. At a minimum, this idea should be
documented somewhere though!
Oh, about the `_coerce_map_from_` hack: I can certainly reimplement
`_coerce_map_from_` on each child class and hard-code the name of the
subclass there. However, this seems to be bad OOP practice--I explicitly
do not want child classes to inherit that method, since it'll introduce a
subtle bug that we both missed at first glance. What I want, and what
seems reasonable, is either for the dynamically generated class to
preserve ``isinstance`` or to get at the non-dynamically generated class,
either of which seems to be an issue with the category framework. But I'm
not at all familiar with the category framework, so I'll defer to experts.
Could you expand on how the current hack "completely breaks the
assumptions of the coercion framework"?
Thanks again for your input!
--
Ticket URL: <http://trac.sagemath.org/ticket/18013#comment:24>
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.