#6097: [with patch, positive 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 nthiery):
Hi Franco!
Replying to [comment:17 saliola]:
> 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.
Oops, that was in my dreams. I only advertised it oraly at SD15. You
should have been there :-)
> 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.
:-)
> You can't really tell from what I wrote in my last comment, but I spent
> almost 3 hours looking at this last night.
Great.
> 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).
Besides the nice visual effect, allowing for automatic checks is precisely
the purpose of @abstract_method.
This is already in #6343 (please update to the latest Sage-Combinat in an
hour or so to get the exact syntax below):
sage: TestSuite(ZZ).run(verbose = True)
running ._test_not_implemented_methods() ... done
running ._test_pickling() ... done
> 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.
That's sensible for Parents, but would not be reasonable for elements that
could be created by the millions.
> 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.)
Note: you can do the same on x.__class__, which is what
.test_not_implemented_methods does
> 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.
I definitely would like this as well! Probably under a different name
though, like
@dummy_method / @default_method, since the purpose is a bit different.
> 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.
Ok. Let's see how this works in practice.
> 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!
I did! Thanks very much for taking the time typing it up.
Cheers,
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/6097#comment:19>
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
-~----------~----~----~----~------~----~------~--~---