#18013: Implement abstract tableau refactoring
-------------------------------------+-------------------------------------
Reporter: jpswanson | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.9
Component: combinatorics | Resolution:
Keywords: days64, sage- | Merged in:
combinat, tableaux, refactoring, | Reviewers: Travis Scrimshaw
days64.25, days68 | Work issues:
Authors: Josh Swanson | Commit:
Report Upstream: N/A | 37ace348fb1afdb930b0657a80e40a58aea14be2
Branch: public/18013 | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Changes (by tscrim):
* reviewer: => Travis Scrimshaw
* milestone: sage-6.6 => sage-6.9
Comment:
The reason it doesn't appear elsewhere is that either it doesn't use
categories/parent/elements (e.g. graphs) or creation of elements are done
only through the parent. There are only a few other examples where I know
of outside of `combinat` that uses parents/elements that uses a function
as a way of creating elements not through the parent. They are `integer`
and `rational`, and they are low-level, optimized, and cython (which until
very recently did not support metaclasses). So it ends up being more of a
"house style" because of what our users expect, but it is a slight abuse
IMO (how many papers say, let `X` be the set of all partitions, and let
`\lambda \in X`?).
For the normalization, I see many places in the code where you seem to be
fighting against doing normalization/validation, most notably: it was part
of your reasoning for the factory functions IIRC, the `check` argument
checks everywhere, the method which iterates over the `dict` of cells and
having that be an abstract method. The last one is the reason I suggested
the refactoring and the ABCs (which can be very complicated, but only
require one or two methods to be implemented in the concrete class;
depends on the situation) would be more minimal. I'm starting to read
deeper into the code and think about the structuring now. (I saw a talk at
last year's ~PyCon which gave a good argument why we should work harder
now rather than take the "technical debt" that we'd have to pay later.)
I'm sorry, I must have misunderstood what you said about the iteration.
For the `_coerce_map_from_`, if you do not want the child classes to
inherit it, then they should not be child classes. If `Y` is a subclass of
`X`, then instances of `Y` should behave like instances of `X` (Nicolas
has referred to this as the 'is a' test). I'm okay with an ABC and/or mix-
in class not conforming to this, but not for user accessible concrete
classes.
I will take a full detailed look at this next week and do my reviewer pass
over the code (which might involve the refactoring I mentioned above).
Thank you again for your work on this.
(Oh look a short-ish response by me :P)
--
Ticket URL: <http://trac.sagemath.org/ticket/18013#comment:25>
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.
