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

Inherit docstrings from parent classes #339

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

has2k1
Copy link
Contributor

@has2k1 has2k1 commented Mar 14, 2024

This change makes it so that subclass methods that do not have
docstrings inherit a docstring from the first parent class whose
overriden method has a docstring.

It also improves the case for the options

include_empty: false
include_inherited: true

where inherited subclass methods are omitted if they have no docstring,
yet a parent class method has a docstring.

e.g.

class Base:
    def m1(): ...
        """Calculate m1"""

    def m2(): ...
        """Calculate m2"""

class Derived(Base):
    def m2(): ...

If the intention is to document Derived and include the inherited
methods, it would be a surprise if Derived.m2 is omitted or it is
included but it has no docstring. As a result to have an option like
include_inherited_docstring that can be false does not align
with the users expected intentions.

This change makes it so that subclass methods that do not have
docstrings inherit a docstring from the first parent class whose
overriden method has a docstring.

It also improves the case for the options

```yaml
include_empty: false
include_inherited: true
```

where inherited subclass methods are omitted if they have no docstring,
yet a parent class method has a docstring.

e.g.

```
class Base:
    def m1(): ...
        """Calculate m1"""

    def m2(): ...
        """Calculate m2"""

class Derived(Base):
    def m2(): ...
```

If the intention is to document `Derived` and include the inherited
methods, it would be a surprise if `Derived.m2` is omitted or it is
included but it has no docstring. As a result to have an option like
`include_inherited_docstring` that can be `false` does not align
with the users expected intentions.
@has2k1
Copy link
Contributor Author

has2k1 commented Mar 14, 2024

The tests that fail were broken by griffe v0.42.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant