#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
-~----------~----~----~----~------~----~------~--~---

Reply via email to