Pylint: Abstract class not overloading an abstract method should be abstract itself.

[pylint-bot]

Originally reported by: Yann Dirson (BitBucket: ydirson, GitHub: @ydirson?)

---

W0223/abstract-method triggers here for class B:

#!python

class A(object):
    def m1(self):
        raise NotImplementedError()

class B(A):
    def m2(self):
        return self.m1()

class C(B):
    def m1(self):
        return 1

It's quite obvious to a human reader that B is abstract, but pylint pylint would need a hint. class_is_abstract apparently only considers a class to be abstract if it declares abstract methods by itself, and in fact, if I add a new abstract method in B, the warning goes away.

I could use an @abstractclass decorator to forcefully add an abstract method with a silly name, but that does not look like a very clean thing to do. Silencing W0223 around the class def would not declare the class to be abstract for other checks.

If only it were checking an is_abstract astroid property on the class, this could be added via a transform plugin eg. on klass.__abstract__ = 1, but with 1.1.0 it looks like there is no other way out than adding a dummy property ?

---

• Bitbucket: https://bitbucket.org/logilab/pylint/issue/179

[pylint-bot]

_Original comment by_ Sylvain Thénault (BitBucket: sthenault, GitHub: @sthenault?):

---

"It's quite obvious to a human reader that B is abstract"

huuuum, not as is, at least.

May abc module support could help here.

Claudiu, any hint on this?

[pylint-bot]

_Original comment by_ Claudiu Popa (BitBucket: PCManticore, GitHub: @PCManticore):

---

Annotating m2 with abc.abstractmethod should be sufficient to mark this class as abstract for Pylint. Otherwise, even as a human reader, without knowing the fact that m1 raises NotImplementedError, there's no hint that B is abstract at all.

[pylint-bot]

_Original comment by_ Sylvain Thénault (BitBucket: sthenault, GitHub: @sthenault?):

---

Can we reject this issue then?

[pylint-bot]

_Original comment by_ Claudiu Popa (BitBucket: PCManticore, GitHub: @PCManticore):

---

Yes.

[pylint-bot]

_Original comment by_ Yann Dirson (BitBucket: ydirson, GitHub: @ydirson?):

---

Problem with the idea of flagging m2 as abstractmethod, is that m2 is not abstract. Doing so would artificially require a override of m2, whereas what's needed is really an override of m1.

And yes, it may not be obvious in more complicated cases that B is still abstract, and I expect that a specific tagging that pylint could grasp would also help a human.

As for my workaround of using a decorator that would add a dummy method that would be seen as abstract by pylint (and decorating any abstract subclasses to overload it with the same defn), it would have the drawback that any tool that correctly propagate the "abstract" status to subclasses when an abstract method is not overloaded would require a specific @concrete to overload it again with a non-abstract version.

So maybe the problem is rather just that pylint should propagate itself the "abstract" status if any of the abstract methods that make one of its parent's abstract are not overloaded.

[pylint-bot]

_Original comment by_ Sylvain Thénault (BitBucket: sthenault, GitHub: @sthenault?):

---

doesn't the abc module provide a way to do so?

[pylint-bot]

_Original comment by_ Yann Dirson (BitBucket: ydirson, GitHub: @ydirson?):

---

Doesn't seem so : metaclass ABCMeta just allows to tell that "this class is probably abstract, and its subclass may as well, but obviously some subclass is going to be concrete (which does not prevent further subclasses of concrete classes to get abstract again)".

Looks like all the info about classes being abstract is really in effective methods of each class, which mean that not only methods defined here should be looked at, but apparently all inherited methods.

[pylint-bot]

_Original comment by_ Sylvain Thénault (BitBucket: sthenault, GitHub: @sthenault?):

---

IMO if your use case isn't supported by abc, I'm not sure we should provide
a way to declare such thing on the pylint side. Disabling the 'abstract
method not implement' messages on the class should be enough, no?

[pylint-bot]

_Original comment by_ Claudiu Popa (BitBucket: PCManticore, GitHub: @PCManticore):

---

There's only one thing I kinda disagree with, marking a method abstract by raising NotImplementedError. In my opinion, this ruins cooperative calling, like in the example below:

#!python

class A:
   def method(self):
      raise NotImplementedError()

class B(A):
    def method(self):
       do something
       return super().method()

Having a method decorated with abstractmethod which does nothing else but pass is more useful than raising NotImplementedError, because you don't know ahead how your methods will be called by others or other subclasses.
In your example, m2 is abstract through side effects, because m1 raises NotImplementedError, so it becomes by itself abstract.

[pylint-bot]

_Original comment by_ Yann Dirson (BitBucket: ydirson, GitHub: @ydirson?):

---

Well, the thing is I'm not even using abc in that project, just the good old "raise NotImplementedError()" idiom. It's right that using abc would provide runtime check - but those require effective instanciation, which is no real replacement for a static check (full unit tests would help too).

If doing the full test looks hard/costly, what about my original suggestion, of allowing plugins to modify pylint's idea of whether a class is abstract ?

[pylint-bot]

_Original comment by_ Sylvain Thénault (BitBucket: sthenault, GitHub: @sthenault?):

---

marking a method abstract by raising NotImplementedError was the only way
to do so before the abc module went in. This is now kept for bw compat
reason (and because it's still imo a fairly common way to define abstract
classes in python). I agree abc has advantages over this method.

Now, I'm not sure how pylint will behave in the situation described by the
OP, though using abc?

More generally, the problem with this code sample is imo that we can't know
if the B class is abstract by intention or simply missing the concrete
implementation of some abstract method...

[pylint-bot]

_Original comment by_ Sylvain Thénault (BitBucket: sthenault, GitHub: @sthenault?):

---

yann, and what about my suggestion of simply disabling the offending
message for the B class ?

[pylint-bot]

_Original comment by_ Claudiu Popa (BitBucket: PCManticore, GitHub: @PCManticore):

---

If m2 is decorated with abstractmethod, then Pylint will flag the method as abstract and it will not complain anymore about abstract-method. That's how is_abstract is implemented anyway. And when instantiating B no one will complain, because it doesn't use ABCMeta as metaclass.

[pylint-bot]

_Original comment by_ Yann Dirson (BitBucket: ydirson, GitHub: @ydirson?):

---

But it m2 is decorated with abstractmethod, C becomes uninstanciable unless m2 is overloaded in C too.

@sthenault: as I wrote initially, silencing W0223 around the class def would not declare the class to be abstract for other checks. Not sure how many other builting checks look at it, but plugins could.

Your point about "abstract by intention or not" is interesting, but when a concrete implementation is missing, either it is missing from the whole hierarchy and the but makes all subclasses abstract (which can be caught by other checks), or it is wrongly implemented in another class (design error, which pylint cannot know about in the general case, but has means to point to through other specific checks).

It also raises an interesting question : even when a class would be marked as voluntarily abstract because of a couple of specific inherited abstract methods, catching missing concrete implementations will only be possible on classes down the hierarchy. It could be nice to be able to annotate a class @abstractbecauseof(methodlist), and only trigger W0223 for methods not in the list (reminds me of one of the rare java idioms I miss in python : "throws" annotations on funcs - but that's a different issue)

[pylint-bot]

_Original comment by_ Claudiu Popa (BitBucket: PCManticore, GitHub: @PCManticore):

---

But it m2 is decorated with abstractmethod, C becomes uninstanciable unless m2 is overloaded in C too.

Not true, only if you use abc.ABCMeta as metaclass. Otherwise, it's just a marker for Pylint to pick that class as abstract.

[pylint-bot]

_Original comment by_ Yann Dirson (BitBucket: ydirson, GitHub: @ydirson?):

---

OK, I was not aware of the ABCMeta-less usage of @abstractmethod, good to know.

But the original problem would be similar if m2 was not calling m1, where it would make no sense then to mark m2 abstract.

[sam-s]

Also discussed on SO: Pylint for half-implemented abstract classes.
Note that even adding __metaclass__ = abc.ABCMeta to the semi-implemented class class B(A) does not solve the Pylint problem.

[svenpanne]

I'm still a little bit confused by this issue. Even after reading through all replies, I'm not sure what the recommended way to handle a hierarchy like this is:

class AbstractClass(object):
    __metaclass__ = abc.ABCMeta

    @abc.abstractmethod
    def foo(self):
        """abstract method one"""

    @abc.abstractmethod
    def bar(self):
        """abstract method two"""


class LessAbstractClass(AbstractClass):
    __metaclass__ = abc.ABCMeta

    def foo(self):
        return 42

Conceptually, the __metaclass__-assignment on LessAbstractClass is not needed, but even with that very explicit hint pylint, complains that bar is not implemented.

What should one do? Adding annotated dummy methods just for pylint's sake can't really be a solution, this confuses readers. The only sane thing I could come up with was suppressing abstract-method in LessAbstractClass, but this is not really satisfying... :unamused:

[teuneboon]

Currently one of the only issues I need the inline # pylint: disable= syntax for. I feel like it's a completely legit use-case to have a class that extends from an abstract class, but is still abstract itself even though it doesn't define any specific abstract methods. Is there a case against a class being considered abstract if it has metaclass=ABCMeta or __metaclass__ = ABCMeta?

[Yevgnen]

Any news in 2019?

[PCManticore]

@Yevgnen No one got around to fix this issue just yet. If this is a problem for you, we'd be happy to accept a patch.