#239 method signature overrides don't get inherited

closed-fixed
nobody
5
2008-01-29
2007-12-21
Anonymous
No

In the docs for the attached module, documentation for Base.method() specifies 'method(self, parameter)' signature, as it is overriden. Derived.method() inherits documentation of its superclass method, but doesn't inherit overriden signature along them.

Of course, this is not an issue for Python methods, but a little annoying for C extension module, where you do have the need to specify signature overrides.

Discussion

  • Nobody/Anonymous

    simple module exhibitng the problem in its docs

     
  • Edward Loper

    Edward Loper - 2008-01-28

    Logged In: YES
    user_id=195958
    Originator: NO

    This is a tricky one -- usually, if a derived method has a signature that disagrees with its base method's signature, then that derived method signature should take precedence. E.g.:

    class A:
    def foo(x): pass

    class B(A):
    def foo(x, y=None): Pass

    That's how epydoc looks at what happens here -- the Derived class method has a different signature than its base class, so that signature takes precedence. I understand that repeating the signature line in all derived methods may be a bit annoying, but I believe that the alternative -- having derived method signatures *not* take precedence -- would be the wrong thing in a majority of cases.

    The only "fix" I can think of would be to add some directive, like "@inheritsignature:" -- but if you're going to have that, then you might as well have the signature itself in the docstring. Besides, it's probably good practice to include the signature in the docstring of C extension module methods, in case someone wants to try to figure out its usage from the interactive command prompt.

    So, unless you can think of a way to fix this issue that doesn't do the wrong thing when derived method signatures are *supposed* to take precedence, I'm going to mark this bug as WorksForMe, with the recommended solution being to specify the signature explicitly in all derived methods. But feel free to make other suggestions, and I'll consider re-opening the bug report.

     
  • Edward Loper

    Edward Loper - 2008-01-28
    • status: open --> pending-works-for-me
     
  • Paul Pogonyshev

    Paul Pogonyshev - 2008-01-28

    Logged In: YES
    user_id=1203127
    Originator: NO

    A fix for the case I'm interested in might be simple. If signature is infered from C extension (AFAIK it is always 'object function(...)'), then this shouldn't override anything. I.e. I'm fine with signature overrides not being inherited for pure Python modules, attached module is actually only an example. I'm more interested in C extension functions/methods.

    In principle, inheriting signature override for C functions might not be correct in some cases. For instance, your derived class method can accept an extra optional argument, etc. But I think practically anything will be an improvement over '...'. And I also guess that in 90% or more cases inheriting will be correct.

     
  • Paul Pogonyshev

    Paul Pogonyshev - 2008-01-28
    • status: pending-works-for-me --> open-works-for-me
     
  • Paul Pogonyshev

    Paul Pogonyshev - 2008-01-28

    Logged In: YES
    user_id=1203127
    Originator: NO

    Besides, the cases when it will be incorrect are the cases when you have incentive to write documentation anyway (i.e. method adding some extra semantics and good documentation should describe them). Unlike, when a derived method does exactly the same (semantically) as the base method, writing documentation is not that needed and signature inheritance is useful _and_ correct.

     
  • Edward Loper

    Edward Loper - 2008-01-28

    Logged In: YES
    user_id=195958
    Originator: NO

    Your point is well taken -- in the limited case of just c-extension methods, where the default is "...", it probably does make sense to inherit. I'll see what I can do to get this implemented.

     
  • Edward Loper

    Edward Loper - 2008-01-28
    • status: open-works-for-me --> open
     
  • Edward Loper

    Edward Loper - 2008-01-29
    • status: open --> pending-fixed
     
  • Edward Loper

    Edward Loper - 2008-01-29

    Logged In: YES
    user_id=195958
    Originator: NO

    Ok, I checked in a change that should do what you want, in svn revision 1665. If you get a chance, please do a checkout of epydoc from svn, and see if it does what you want. As it's implemented now, it will only apply if the inherited & overridden method both satisfy inspect.isbuiltin(). In particular, it won't apply for method descriptors, since I can't tell if those come from c extension modules or not.

    For instructions for getting epydoc from svn, see:

    http://epydoc.sourceforge.net/installing.html

    Thanks.

     
  • Paul Pogonyshev

    Paul Pogonyshev - 2008-01-29
    • status: pending-fixed --> open-fixed
     
  • Edward Loper

    Edward Loper - 2008-01-29

    Logged In: YES
    user_id=195958
    Originator: NO

    Hm, that's disappointing. If you run inspect.isbuiltin() on the methods in question (both the overriding method and the overridden method), do they return true? If not, then what do they return for the other inspect.is*() methods?

    (The regression may come from another bug that I was trying to fix: https://sourceforge.net/tracker/index.php?func=detail&aid=1879931&group_id=32455&atid=405618 )

    p.s., thanks for the patch removing the generator expression -- I try to maintain compatibility with py2.3+, but I don't always remember to check changes on older python versions. Applied as svn revision 1682.

     
  • Paul Pogonyshev

    Paul Pogonyshev - 2008-01-29

    Logged In: YES
    user_id=1203127
    Originator: NO

    It seems inspect.ismethoddescriptor() can differ C extension and pure Python functions. However, can't say I like this since I don't really understand what it does :-/ Maybe you do, though ;)

    >>> import inspect
    >>> from notify.gc import *
    >>> inspect.isbuiltin (AbstractGCProtector.protect)
    False
    >>> inspect.isbuiltin (FastGCProtector.protect)
    False
    >>> from notify.condition import *
    >>> inspect.ismethoddescriptor (FastGCProtector.protect)
    True
    >>> inspect.ismethoddescriptor (AbstractGCProtector.protect)
    True
    >>> inspect.ismethoddescriptor (Condition.set)
    False

    Note that Condition class is pure Python, so we indeed seem to have a division here.

     
  • Edward Loper

    Edward Loper - 2008-01-29

    Logged In: YES
    user_id=195958
    Originator: NO

    Method descriptors can be defined in pure python [1] -- they're basically just a class instance that defines __get__ but not __set__. I installed your package, and after playing around with it a little, it looks like the best solution is to test the type of the method -- if it's <type 'method_descriptor'> (aka type(list.append)), then it counts as a builtin method. This will include c extension method descriptors, but exclude pure python method descriptors, which is what we want.

    Checked in as svn revision 1683. It appears to work for me when I run it, but if you have any further problems with it, please re-open this bug report.

    [1] http://users.rcn.com/python/download/Descriptor.htm

     
  • Edward Loper

    Edward Loper - 2008-01-29
    • status: open-fixed --> closed-fixed
     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

JavaScript is required for this form.





No, thanks