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

Request: not throwing DOC501 for abstractmethod #100

Open
jamesbraza opened this issue Oct 24, 2023 · 3 comments
Open

Request: not throwing DOC501 for abstractmethod #100

jamesbraza opened this issue Oct 24, 2023 · 3 comments

Comments

@jamesbraza
Copy link
Contributor

from abc import ABC, abstractmethod


class Foo(ABC):
    @abstractmethod
    def bar(self, arg: int = 1) -> None:
        """
        Do something.

        Args:
            arg: Some argument.
        """
        raise NotImplementedError


class DFoo(Foo):
    def bar(self, arg: int = 1) -> None:
        print(arg)  # NOTE: subclass doesn't have potential to raise a NotImplementedError

For abstractmethod, it's a common practice to make the base class's body be raise NotImplementedError. This basically denies subclasses from calling super(), double enforcing the override.

Currently, as of pydoclint==0.3.8, it throws DOC501 on the base class's docstring for a missing "raise" statement (per the NotImplementedError). However, subclasses won't have the NotImplementedError in their implementation, so this is sort of a false positive DOC501.

I think pydoclint should not be throwing DOC501 when it's a docstring for an abstractmethod in an ABC.

@jsh9
Copy link
Owner

jsh9 commented Nov 11, 2023

I don't think I fully understand your question.

If Foo.bar()'s body raises an exception, we need to document it in its docstring.

And if DFoo.bar()'s body does not raise any exceptions, we don't need to document it in its docstring.

@jamesbraza
Copy link
Contributor Author

Before I answer, I want to understand one related scenario for DOC501. Let's say a parent class's method raises an Exception. If:

  • The child's overridden method calls super(), and otherwise doesn't throw an Exception: will DOC501 be thrown?
  • The child's overridden method doesn't call super(), and doesn't throw an Exception: will DOC501 be thrown?

@llucax
Copy link
Contributor

llucax commented Jul 26, 2024

I would also like to second this request.

If Foo.bar()'s body raises an exception, we need to document it in its docstring.

In this case the exception is just an artifact, since Python is a dynamic language, there is no way to enforce implementing an abstract method statically, so you need to raise an exception at runtime.

I also think that documenting the NotImplementedError is noisy and pointless. As a developer, it is enough to know that a method is abstract to know I shouldn't call it, is not like I need to handle the NotImplementedError in any way.

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

No branches or pull requests

3 participants