Skip to content

pydoc renders from builtins.type note, even if it is incorrect #97959

Description

@sobolevn

While working on #97958 I've noticed that there's something strange with help() and classmethods.

Take a look at this example:

import pydoc

class My:
    @classmethod
    def __init_subclass__(cls, *args, **kwargs):
        pass

    @classmethod
    def custom(cls):
        pass

print(pydoc.plain(pydoc.render_doc(My)))

It prints:

Python Library Documentation: class My in module __main__

class My(builtins.object)
 |  Class methods defined here:
 |
 |  __init_subclass__(*args, **kwargs) from builtins.type
 |      This method is called when a class is subclassed.
 |
 |      The default implementation does nothing. It may be
 |      overridden to extend subclasses.
 |
 |  custom() from builtins.type
 |
 |  ----------------------------------------------------------------------
 |  Data descriptors defined here:
 |
 |  __dict__
 |      dictionary for instance variables (if defined)
 |
 |  __weakref__
 |      list of weak references to the object (if defined)

Take a look at these two entries:

  1. __init_subclass__(*args, **kwargs) from builtins.type
  2. custom() from builtins.type

While type has __init_subclass__, there's no type.custom. But, help says that there is!

>>> type.__init_subclass__
<built-in method __init_subclass__ of type object at 0x10a50c360>
>>> type.custom
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: type object 'type' has no attribute 'custom'

I think that it is incorrect and can lead to confusion.
Instead it should be:

 |  __init_subclass__(*args, **kwargs) from builtins.type
 |      This method is called when a class is subclassed.
 |
 |      The default implementation does nothing. It may be
 |      overridden to extend subclasses.
 |
 |  custom()

Linked PRs

Activity

  1. sobolevn commented on Oct 6, 2022

    @sobolevn
    MemberAuthor

    A couple of extra thoughts after reading some more source code:
    https://github.com/python/cpython/blame/e39ae6bef2c357a88e232dcab2e4b4c0f367544b/Lib/pydoc.py#L1032-L1042

            if _is_bound_method(object):
                imclass = object.__self__.__class__
                if cl:
                    if imclass is not cl:
                        note = ' from ' + classname(imclass, mod)

    Ok, looks like this is in sync with the following example:

    import pydoc
    from types import MethodType
    
    class My:
        some = MethodType(some, 1)
    
    print(pydoc.plain(pydoc.render_doc(My)))

    which outputs:

    Python Library Documentation: class My in module __main__
    
    class My(builtins.object)
     |  Methods defined here:
     |
     |  some(*args, **kwargs) from builtins.int
    

    I think that classmethods should just completely skip this part.
    It brings more confusion than value. So, it should be:

    |  __init_subclass__(*args, **kwargs)
     |      This method is called when a class is subclassed.
     |
     |      The default implementation does nothing. It may be
     |      overridden to extend subclasses.
     |
     |  custom()
    
  2. added
    stdlibStandard Library Python modules in the Lib/ directory
    on Oct 6, 2022
  3. added a commit that references this issue on Oct 9, 2022
  4. serhiy-storchaka commented on Oct 10, 2022

    @serhiy-storchaka
    Member

    Good catch.

    Why this "from ..." was added in the first case? In what cases it was useful.

  5. sobolevn commented on Oct 10, 2022

    @sobolevn
    MemberAuthor

    I found only one: when MethodType is defined in a class body:

    class Some:
        a = MethodType(some_func, some_instance)

    Plus, Enum uses from enum.EnumType on some methods. I am still testing other things.

  6. serhiy-storchaka commented on Oct 10, 2022

    @serhiy-storchaka
    Member

    I found only one: when MethodType is defined in a class body:

    But where is it used in the real code?

    If make this code raising an error instead of adding "from ..." and run pydoc for all stdlib modules, what will fail? Investigate also the history of that code, commit message or issue text can explain its purpose.

  7. sobolevn commented on Nov 7, 2022

    @sobolevn
    MemberAuthor

    So, I got back to it. Here's what I found.

    test results

    Running pydoc tests with raise ValueError instead of note = ' from ' + classname(imclass, mod) showed this:

    2 tests failed:
        test_enum test_pydoc
    

    Only pydoc itself and enum are affected.
    Basically, the same modules I've changed in #98120

    history

    There are two commits that touch this line:

    1. 15 years ago, when unbound methods were removed: ff73795
    2. 22 years ago, when b7a4830 was merged. It does not have a description. Its title is Fix linking to classes (in class tree, and add links on unbound methods)

    So, I can conclude that these lines have something to do with old unbound methods.

    = MethodType() usage

    There are some usage of it:

    » ag '= MethodType' 
    Lib/test/test_importlib/import_/test_caching.py
    60:        mock.load_module = MethodType(load_module, mock)
    
    Lib/test/test_call.py
    754:                meth = MethodType(func, args[0])
    

    But, lets think about whether is this important to show from ... part. Compare these two (with and without from):

    >>> import types
    >>> class A:
    ...    x = types.MethodType(some, 1)
    ... 
    >>> help(A)
    Help on class A in module __main__:
    
    class A(builtins.object)
     |  Methods defined here:
     |
     |  x = some(*args, **kwargs) from builtins.int
     |
     |  ----------------------------------------------------------------------
     |  Data descriptors defined here:

    Without:

    >>> help(A)
    Help on class A in module __main__:
    
    class A(builtins.object)
     |  Methods defined here:
     |
     |  x = some(*args, **kwargs)
     |
     |  ----------------------------------------------------------------------
     |  Data descriptors defined here:
    1. Has anything really changed?
    2. Will users be able to decrypt that x is a MethodType bound to int from this short message? I don't think so. I've spent several hours trying to decode what it means and how it works.

    lib usage

    Let's take a look at the enum case: it kinda is using this feature.

     |  ----------------------------------------------------------------------
     |  Methods inherited from enum.EnumType:
     |
     |  __contains__(value) from enum.EnumType
     |      Return True if `value` is in `cls`.
     |
     |      `value` is in `cls` if:
     |      1) `value` is a member of `cls`, or
     |      2) `value` is the value of one of the `cls`'s members.
     |
     |  __getitem__(name) from enum.EnumType
     |      Return the member matching `name`.
     |
     |  __iter__() from enum.EnumType
     |      Return members in definition order.
     |
     |  __len__() from enum.EnumType
     |      Return the number of members (no aliases)
     |
     |  ----------------------------------------------------------------------

    Is it useful here? Again, I don't think so: we already have Methods inherited from enum.EnumType: part which clearly says where these methods come from.

    In my opinion, it brings more confusion than value.

    solution

    I think that we should at least protect @classmethod from been reported incorrectly, as I did in #98120

    Or we can completely remove note = ' from ' + classname(imclass, mod) part, because:

    1. It is an artifact from another era: when unbound methods were a thing
    2. It does not bring any value to existing Lib/ code
    3. It is quite cryptic to understand for regular users
    4. It is easy to do, nothing will probably break because of that

    @serhiy-storchaka what do you think? :)

  8. ethanfurman commented on Dec 9, 2022

    @ethanfurman
    Member

    I do not see any value in the from some_module.some_type. I'd like to see it removed.

  9. ethanfurman commented on Dec 9, 2022

    @ethanfurman
    Member

    Not sure if this is related, but help(enum.Enum) shows:

    Help on class Enum in module enum:
    
    class Enum(builtins.object)
     |  . . .
    

    It seems to me that instead of builtins.object that should say enum.EnumType.

  10. added a commit that references this issue on Jan 11, 2024
  11. serhiy-storchaka commented on Jan 11, 2024

    @serhiy-storchaka
    Member

    This issue is much much more complex than it looked at first glance. If you look at the code, you will see other issues with notes for routines. For example an "unbound method" note is never added to unbound methods. It does not work with methods of builtin classes. It seems that this code was written to handle classic classes, and the support of new-style classes was never complete. Some changes was made later to fix runtime errors, but it was not checked that the result is correct and that all cases are supported.

    Other issue, is that class members that are bound methods were included in the methods section. But they behave rather like static methods, because they do not accept the self argument.

    #113941 fixes these and other issues.

    There are still unfixed issues, for example nested classes are not supported (#85799), and unbound methods of builtin classes are not visible as module members (#113942), but this PR is already large.

  12. added 5 commits that reference this issue on Feb 11, 2024
  13. serhiy-storchaka commented on Feb 11, 2024

    @serhiy-storchaka
    Member

    About the enum.Enum case. It was:

      |  Methods inherited from enum.EnumType:
      |
      |  __contains__(value) from enum.EnumType
      |      Return True if `value` is in `cls`.
    

    First, "from enum.EnumType" is redundant, because there is "from enum.EnumType" in the section title. Usually "from" notes are not repeated for every method.

    Second, the section title is not completely correct. They are not normal methods. The first argument of normal arguments is self (whatever its name). You cannot call enuminstance.__contains__(). You should call enumclass.__contains__(value) -- this is the behavior of static or class method. It is difficult to distinguish them at that level, so for now they are qualified as static methods.

    BTW, for the "from" example look at the enum.StrEnum help. Now it contains:

     |  Methods defined here:
    ...
     |  __repr__(self) from enum.Enum
     |      Return repr(self).
     |
     |  __str__(self, /) from builtins.str
     |      Return str(self).
    

    Note that __str__ is "from builtins.str", and __repr__ is "from enum.Enum", despite both are "methods defined here". You can see that __str__ is the same as for strings, and __repr__ is implemented in Enum, and that they override methods inherited from base classes.

  14. added a commit that references this issue on Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    stdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions