#19609: Added image and kernel methods to morphisms of finite-dimensional
modules
with basis
-------------------------------------+-------------------------------------
Reporter: tscrim | Owner: sage-combinat
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-6.10
Component: linear algebra | Resolution:
Keywords: kernel, image, | Merged in:
module morphism | Reviewers:
Authors: Travis Scrimshaw | Work issues:
Report Upstream: N/A | Commit:
Branch: | fd8d546eaeeb845b5b610f946ce7ab27e77c4095
public/categories/image_kernel_module_morphism-19609| Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Comment (by tscrim):
Replying to [comment:13 darij]:
> Hmm, I'm not sure I'm happy with this -- but once again, this seems to
be a Sage design flaw, not an error of your branch. Now that we are
overriding `__eq__`, everyone implementing a new class of module morphism
will have to override `__eq__` as well or risk distinct morphisms
comparing as equal.
That is how you should write code in good OOP, inherit and use the parent
class methods and override if necessary. I certainly understand why
comparing the `__dict__` was done and it was a good solution at the time.
> Maybe we should also compare `on_basis()` and `_inverse` on triangular
module morphisms?
`_inverse` is completely dependent on the other attributes, so I think
that is a wasted test. However, the current inheritance scheme does not
explicitly have an `on_basis` in `TriangularModuleMorphism`. Strictly
speaking, I'm not opposed to doing this, but by the current class
hierarchy, it would get double-checked because `TriangularModuleMorphism`
is an ABC and the actual concrete implementations will end up checking
that. So on that, I would say your test:
{{{
TriangularModuleMorphism.__eq__(phi, phi2) # I don't like this :/
}}}
is misleading. It is the correct result because as far as that class is
concerned, they are equal (in fact, IMO the call to `on_basis` in the
`__init__` should be moved into the `if inverse_on_support == "compute":`
block since that is the only place it is used).
Actually, it might be better to remove the inheritance from
`ModuleMorphism` and have `TriangularModuleMorphism` as a mixin class.
However all of this should be on a separate ticket. I can also split the
`__eq__` part off to said ticket as it is not needed for the features I
was originally after.
> Nicolas, Simon, or whoever else understands these matters: any chance
you could look over the `__eq__` definitions?
I would appreciate your (Nicolas and/or Simon) comments on these classes
and what you think we should do moving forward. On a somewhat related
question, do you think we should spend time to Cythonize these classes?
--
Ticket URL: <http://trac.sagemath.org/ticket/19609#comment:14>
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 https://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
