#6097: [with patch, needs review] Implements a mantra for declaring abstract
methods
-------------------------+--------------------------------------------------
Reporter: nthiery | Owner: nthiery
Type: enhancement | Status: assigned
Priority: major | Milestone: sage-4.1
Component: misc | Keywords: abstract methods
Reviewer: | Author: nthiery
Merged: |
-------------------------+--------------------------------------------------
Comment(by saliola):
Hello,
Replying to [comment:16 nthiery]:
> Replying to [comment:15 saliola]:
> > Here is a second opinion.
> > I really, really like the idea of adding such a decorator! And for the
> > record, under normal circumstances, I would have asked for a full
design
> > discussion to take place on sage-devel since I share many of the above
> > concerns and because I think that there are many subtle design issues
> > that need to be addressed. (We'll have to have that discussion later,
> > it seems.)
>
> I tried to advertise the feature to start such a discussion, but there
was not much reaction. I think we first need to accumulate experience by
using the current thing, and then discuss again. Release early, release
often.
I definitely missed this. I'm behind in reading the sage-devel emails.
> > Here is a list of things that I don't like (most have been mentioned
above):
> >
> > - raising {{{NotImplementedError}}} when an abstract method is
accessed
> > - the error message is not useful
> > - returning {{{NotImplemented}}} for an optional abstract class
> > - can't access the documentation for an abstract method
> > - that {{{hasattr(x, 'my_method')}}} returns False for abstract
methods
> > and True for optional abstract methods
> > - can't determine what the undefined abstract methods of an object
are
>
> Thanks for the feedback! I am also not yet happy about the current
state.
You mentioned that in the doc and in the above comments, and I must admit
that this is one of the things that makes me more comfortable with
including this into Sage. I know that you'll be around to improve it in
the
future, and that the design isn't complete yet.
> So do you have suggestions for what to return in both cases?
> What mantra would you like to see to test whether an optional abstract
method has, or not,
> been implemented?
You can't really tell from what I wrote in my last comment, but I spent
almost 3 hours looking at this last night.
I thought about what I like and dislike about the existence of such a
decorator. I really like the idea, but found myself wondering how much
better is this, really, than good documentation? Will this encourage a
developer to be lazy in documenting what needs to be implemented by
subclasses? I strongly suspect this will be the case, which is fine as
long
as there is support for easily determining which methods of a class need
to
be implemented (I don't want to have to grep for {{{abstract_method}}} in
the source). I wondered how I would use this as a developer of a subclass?
I mean, given an object {{{x}}}, is there a way to tell which abstract
methods of {{{x}}} need to be implemented without grepping the source?
This
is where introspection would normally come in (see below). And, as the
developer of the subclass, it would be best to have {{{x.my_method?}}}
return a docstring that says what the implementation should do (this is
already mentioned in the docstring as a future improvement).
I wondered about, as ncalexan suggested, checking whether abstract methods
are implemented at instance creation. This is exactly what happens now
with
{{{CombinatorialAlgebra(QQ)}}}, but I find that this puts too much power
in
the hands of the developer since a user might want to use a class for
something that doesn't require a particular abstract method at all.
I wondered about different mantras. If no error is returned on accessing
elements, then one can use introspection to find all the abstract methods
of an object.
{{{
sage: [meth for meth in dir(x) if type(x.meth, AbstractMethod)]
}}}
(The above currently doesn't work because {{{x.my_method}}} raises an
error.)
I also wondered whether some methods might be decorated with
{{{abstract_method}}}, but have a default implementation. I'm thinking of
a
decorator that signals to the user/developer that even though that
particular method is implemented, it would be vastly better (for
speed/memory/...) to provide a better implementation if one exists. I'm
think about some of the methods of
{{{CombinatorialClass}}}/{{{EnumeratedSet}}}, in particular. Perhaps this
is beyond the scope of this decorator though, since an abstract method
seems to be just dummy code usually.
Personally, I am leaning towards the idea of raising a
{{{NotImplementedError}}} upon calling the method instead of upon
accessing
it, together with a good error message that tells the user that this
method
needs to be defined in order for what they just asked for to work. I
haven't thought through this enough to be convinced that this is my
preferred approach, and there might be some fine points I haven't thought
of that makes this unappealing.
So, lots of thoughts, not many conclusions.
I decided to keep my last post short and to the point because I knew this
type of post would be lengthy, but you asked!
> Feel free to include those suggestions in the doc via a reviewers patch,
so that users run into them in the future.
I think it isn't necessary since you have adequately stated in the
documentation that it is just the beginning. And it also lists several
todo's, one of which is to scour the web for other implementations/ideas
and merge them. Anyhow, as I said above, I have arrived at no conclusions
yet, just preferences.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/6097#comment:17>
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
-~----------~----~----~----~------~----~------~--~---
