Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parent class's __init__ docstring shown when child has a class docstring #1047

Closed
Prunoideae opened this issue Mar 14, 2021 · 6 comments
Closed
Assignees
Labels
bug Something isn't working docstrings fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@Prunoideae
Copy link

Prunoideae commented Mar 14, 2021

Environment data

  • Language Server version : 2021.3.1
  • OS and version: Windows 10 Pro Insider Preview Build 21313
  • Python version (& distribution if applicable, e.g. Anaconda): 3.9.1 64-bit

Expected behaviour

class foo():
    '''
    Hello
    '''

    def __init__(self) -> None:
        pass

foo()

In this case Hello should be displayed when cursor is hovered on foo().

Actual behaviour

image
image

The very first doc of __init__ is displayed, other than the Hello, if we add '''Hello''' in the __init__'s spec then it will be displayed. And if we remove the __init__ definition, the class's docstring will be displayed.

This may cause inconvenience when reviewing large code blocks with objects, since docstrings are not directly displayed, and the case is inevitable even in packages like pandas because most code will not add same, or independent docstring for __init__, and it's repetitive if such a docstring is required other than a class's own docstring.

@judej judej added the needs investigation Could be an issue - needs investigation label Mar 15, 2021
@judej
Copy link
Contributor

judej commented Mar 15, 2021

Will investigate showing the most appropriate docstring. Thanks for the report.

@github-actions github-actions bot removed the triage label Mar 15, 2021
@jakebailey jakebailey added bug Something isn't working docstrings labels Aug 30, 2021
@jakebailey jakebailey changed the title docstrings of classes displayed abnormally after __init__ is defined Parent class's __init__ docstring shown when child has a class docstring Sep 2, 2021
@jakebailey
Copy link
Member

jakebailey commented Sep 7, 2021

The next release changes our docstring code to ignore object's docs for classes that aren't object itself. That will fix the above example:

image

... but won't fix something like this:

class A:
    """Class doc for A"""
    def __init__(self):
        """__init__ doc for A"""

class B(A):
    """Class doc for B"""

B()

As we search down the __init__ chain first, finding A.__init__ and using its doc, when we should probably be using B.__doc__.

image

Fixing this is a much more involved change.

@jakebailey
Copy link
Member

This also begs the question of whether or not the class doc is the most appropriate doc in the above example; the method doc may have useful information on how to call it that the class doc would effectively hide. I'm thinking this issue was really only reported due to the 2021.3.0 fixing #550/#877, and previously 2020.9.6 added support for the builtin's docstrings (including object's pesky docs).

Perhaps the "right" solution is to call this fixed and see if anyone believes we should be showing B's doc in the example above later.

help shows both docs, since it's just listing everything, so that's not a useful comparison.

Help on class B in module __main__:

class B(A)
 |  Class doc for B
 |
 |  Method resolution order:
 |      B
 |      A
 |      builtins.object
 |
 |  Methods inherited from A:
 |
 |  __init__(self)
 |      __init__ doc for A

@jakebailey jakebailey added fixed in next version (main) A fix has been implemented and will appear in an upcoming version and removed needs investigation Could be an issue - needs investigation labels Sep 7, 2021
@jakebailey
Copy link
Member

Yeah, I'm going to call this fixed for now. It seems like it's more consistent to not offer this doc if a parent __init__ has one.

@jakebailey
Copy link
Member

This issue has been fixed in version 2021.9.1, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/main/CHANGELOG.md#202191-8-september-2021

@leinaxd
Copy link

leinaxd commented Jun 28, 2022

EDIT: NO BUG. very tricky

This Bug isn't fixed yet. isn't it?
i've also try with pytorch nn.Module, same bug for the same reason

Excellent test case by the way!

Oh, sorry... i understand why... now

  • B() is calling an __init__ function
  • B is the class

So it makes sense linting to __init__ when B() is called instead of class B

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working docstrings fixed in next version (main) A fix has been implemented and will appear in an upcoming version
Projects
None yet
Development

No branches or pull requests

4 participants