#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.

Reply via email to