#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 robertwb):
Replying to [comment:6 nthiery]:
> 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'll second Georg's comments, you are as much of an "expert" in this area
as nearly anyone else (except maybe me, but I can't review it myself...).
> I am now on my way to run the tests, and see how this patch interact
with the category patches.
I saw some tests failing--looks like easy fixes, I'll try and post a patch
later tonight.
> 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"?
I'll change that to say "not implemented here."
> 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?
This can be done, kind of, if one knows the baseclass where the "abstract"
method is implemented, though it's a bit hackish. I think you
underestimate how fast calling a c(p)def method can be though.
> But I don't mind if this waits for a later patch.
Yeah, that'd take some thinking, probably best put off 'till later.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/5596#comment:8>
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
-~----------~----~----~----~------~----~------~--~---
