#5596: [with patch, needs review] refactor coercion to catch fewer exceptions
----------------------+-----------------------------------------------------
Reporter: robertwb | Owner: robertwb
Type: defect | Status: new
Priority: major | Milestone: sage-4.1.2
Component: coercion | Keywords:
Reviewer: | Author:
Merged: |
----------------------+-----------------------------------------------------
Comment(by nthiery):
Thanks for working on this. This will be a big step forward in
debuggability or coercion problems!
In particular, reducing the amount of poking around "let's try if this
method does not break horribly on this element" is
definitely going in the right direction.
I just went through the patch, and each change sounds sensible. I haven't
checked that they are 100% correct, by lack of expertise.
In particular, I haven't checked line by line the big chunks of diffs
corresponding to indentation changes (inclusion in a try).
Overall, +1 for setting positive review, but I would prefer if some expert
could double check.
I am now on my way to run the tests, and see how this patch interact with
the category patches.
One comment:
In Sage-Combinat, we often have operations which are partially defined,
and return None when they are not. How does this fit with the
change in the specifications of _mul_ and friends, suggesting to return
None rather than raising NotImplemented?
Should "Returning None indicates that this action is not implemented" be
replaced by something like:
"Returning None indicates that this action is not defined, or not
implemented here, for the given elements"?
Alternatively, I could possibly suggest to instead check that self._lmul
(say) is
not the default "NotImplemented" implementation of Element, before
actually calling it. I guess I am trying to have an analogue of
the abstract_method idiom for cpdef methods. Would it be possible to
attach some sort of "abstract method" flag to a cpdef method,
that we could test without actually having to execute the method?
But I don't mind if this waits for a later patch.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/5596#comment:6>
Sage <http://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 post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en
-~----------~----~----~----~------~----~------~--~---
