Abstract OOP class / method inheritance / analyzing misbehaving

[clauspruefer]

Bug description

get_value_by_property_id() is not defined as abstract method (@abc.abstractmethod decorator) but pylint treats like.

import abc

class BaseHandler(metaclass=abc.ABCMeta):
    """ Abstract Base Class (ABC) Meta Class.
    """
    def __init__(self):
        self.logger = logging.getLogger(__name__)

    @abc.abstractmethod
    def _add_class(self):
        """ Abstract _add_class() method.
        """
        pass

    def get_value_by_property_id(self, property_id):
        raise NotImplementedError


class ClassHandler(BaseHandler):
    """ ClassHandler class. Inherits BaseHandler class.
    """

    def __init__(self):
        super().__init__()

Configuration

name: Pylint

on: [push]

jobs:
  build:
    runs-on: ubuntu-latest
    strategy:
      matrix:
        python-version: [ "3.10" ]
    steps:
    - uses: actions/checkout@v4
    - name: Set up Python ${{ matrix.python-version }}
      uses: actions/setup-python@v3
      with:
        python-version: ${{ matrix.python-version }}
    - name: Install dependencies
      run: |
        python -m pip install --upgrade pip
        pip install pylint
        pip install microesb

Command used

pylint $(git ls-files 'src/microesb.py')

Pylint output

Method get_value_by_property_id is abstract in class BaseHandler but is not overridden in child class 'ClassHandler'.

Expected behavior

No Exception should be raised by pylint.

Pylint version

pylint-3.3.3-py3-none-any.whl

OS / Environment

GitHub Actions / Docker

Additional dependencies

[clauspruefer]

I forgot to mention: a base abstract class must be "empty" (no code). This also should be analyzed by pylint.

@abc.abstractmethod
    def methodname(self):
        pass

[mbyrnepr2]

It looks like a duplicate of #9275.
The Python docs on NotImplementedError mention that it can be used for abstract methods.

[mbyrnepr2]

I forgot to mention: a base abstract class must be "empty" (no code). This also should be analyzed by pylint.

@abc.abstractmethod
    def methodname(self):
        pass

That would contradict the Python docs for abstractmethod:

Note Unlike Java abstract methods, these abstract methods may have an implementation. This implementation can be called via the super() mechanism from the class that overrides it. This could be useful as an end-point for a super-call in a framework that uses cooperative multiple-inheritance.

[clauspruefer]

I forgot to mention: a base abstract class must be "empty" (no code). This also should be analyzed by pylint.
@abc.abstractmethod
def methodname(self):
pass

That would contradict the Python docs for abstractmethod:

Note Unlike Java abstract methods, these abstract methods may have an implementation. This implementation can be called via the super() mechanism from the class that overrides it. This could be useful as an end-point for a super-call in a framework that uses cooperative multiple-inheritance.

This is 100% true, i forgot the super() call possibility to add some abstract global-ness. This even makes sense for non-multi-inheritance where you have a global routine for every overloaded method.

[clauspruefer]

It looks like a duplicate of #9275. The Python docs on NotImplementedError mention that it can be used for abstract methods.

No, if i would declare get_value_by_property_id() method abstract (meta class method), i had to overload every child class method and this makes the NotImplementedError useless (base method never called).

[mbyrnepr2]

Sorry I think I was too vague! I didn't mean to suggest mixing NotImplementedError with abc.abstractmethod in the same method definition.

What I said was:

The Python docs on NotImplementedError mention that it can be used for abstract methods.

Let me restate that another way.
You can define abstract methods using either of the two approaches in your example. I.e.:

Approach 1

class Base(abc.ABC):
    @abc.abstractmethod
    def say_hello(self):
        print("hello")

Approach 2

class Base:
    def say_hello(self):
        raise NotImplementedError

My feeling of your expected behaviour for Approach 2 (No Exception should be raised by pylint.) is that this would be a contradiction to the Python docs. I think the NotImplementedError approach pre-dates the abc.abstractmethod approach, but they are both still documented and in use in codebases. Maybe it's a good idea to ask on Python Discourse Help to pitch the idea if you think it should be changed?

For reference, the Python docs have this:
NotImplementedError
This exception is derived from RuntimeError. In user defined base classes, abstract methods should raise this exception when they require derived classes to override the method, or while the class is being developed to indicate that the real implementation still needs to be added.

abc.abstractmethod

[clauspruefer]

Approach 2

class Base:
    def say_hello(self):
        raise NotImplementedError

My feeling of your expected behaviour for Approach 2 (No Exception should be raised by pylint.) is that this would be a contradiction to the Python docs. I think the NotImplementedError approach pre-dates the abc.abstractmethod approach, but they are both still documented and in use in codebases. Maybe it's a good idea to ask on Python Discourse Help to pitch the idea if you think it should be changed?

We are getting closer, i will write some detailed comments later on.

Coming tonight ...

[clauspruefer]

Code Example 1 (correct implementation)

import abc


class DecoratorClass():
    def global_func(self, arg1):
        print(arg1)


class Base(DecoratorClass, metaclass=abc.ABCMeta):

    def __init__(self):
        print('init some math')

    @abc.abstractmethod
    def calc_shape(self, x, y, z):
        pass

    def calc_more(self, data):
        raise NotImplementedError


class Shape1(Base):

    def __init__(self):
        super().__init__()

    def calc_shape(self, x, y, z=None):
        print('x-cord:{} y-cord:{}'.format(x, y))


class Shape2(Base):

    def __init__(self):
        super().__init__()

    def calc_shape(self, x, y, z):
        print('x-cord:{} y-cord:{} z-cord:{}'.format(x, y, z))


s1 = Shape1()
s1.calc_shape(10, 20)
s1.global_func('global1')

try:
    s1.calc_more(100)
except NotImplementedError as e:
    print('Correct: NotImplementedError raised.')

s2 = Shape2()
s2.calc_shape(20, 30, 40)
s2.global_func('global2')

try:
    s2.calc_more(200)
except NotImplementedError as e:
    print('Correct: NotImplementedError raised.')

Python 3.11.2 executes the code (working like expected).

python3 test-abstract.py 
init some math
x-cord:10 y-cord:20
global1
Correct: NotImplementedError raised.
init some math
x-cord:20 y-cord:30 z-cord:40
global2
Correct: NotImplementedError raised.

Code Example 2 (incorrect implementation)

I omit the calc_shape() method in Shape2 class.

class Shape2(Base):

    def __init__(self):
        super().__init__()

Python prints out the correct Exception.

    s2 = Shape2()
         ^^^^^^^^
TypeError: Can't instantiate abstract class Shape2 with abstract method calc_shape

Assumption

I assume there is a slight misunderstanding of how abstract class methods work. An abstract class must be overloaded in every child class method to guarantee a different implementation.

An abstract class inheriting from abc.ABCMeta gets base functionality __subclass__, etc. overloaded, but not all methods get flagged abstract. This only applies when decorated by the @abc.abstractmethod decorator.

Wrong pylint Output / Processing

pylint 2.16.2
W0223: Method 'calc_more' is abstract in class 'Base' but is not overridden in child class 'Shape1' (abstract-method)

It would be only valid for Code Example 2 (where the calc_shape() method is flagged abstract in the base and no calc_shape() method exists in the child).

[mbyrnepr2]

I agree that the NotImplementedError approach to creating abstract methods has different behaviour to using abc.abstracmethod; as your examples show.

My point is that both these approaches are documented Python behaviour for creating abstract methods. Therefore Pylint is complete in the sense that it analyses both approaches.
These days you probably want to use abc.abstractmethod in code but NotImplementedError still exists in the language.

We can leave this open to see if anyone wants to voice their thoughts.

[clauspruefer]

Yes, the Python documentation is wrong!

abstract methods should raise this exception when they require derived classes to override the method ...

should be

any non-abstract method should raise this exception when derived classes require to override the method to indicate that the real implementation still needs to be added.

The following example code can be used to clarify this to Python staff (not working like described in the current NotImplementedError documentation). Until progress, this ticket can be closed (and i have to disable the W0223 in my pylint config).

Example 1 (abstract method not declared)

import abc


class Base(metaclass=abc.ABCMeta):

    def test_method(self):
        pass


class Child1(Base):

    def test_method(self):
        print('overloaded in Child1')


class Child2(Base):

    def some_other_method(self):
        pass


c1 = Child1()
c2 = Child2()

Python behavior: No Exception ✅ Pylint: No message raised ✅

Example 2 (abstract method not overloaded, implementation error)

import abc


class Base(metaclass=abc.ABCMeta):

    @abc.abstractmethod
    def test_method(self):
        pass


class Child1(Base):

    def test_method(self):
        print('overloaded in Child1')


class Child2(Base):

    def some_other_method(self):
        pass


c1 = Child1()
c2 = Child2()

Python behavior: TypeError: Can't instantiate ... Exception ✅ Pylint: W0223: Method 'test_method' is abstract ✅

Example 3 (NotImplementedError raising method declared abstract)

import abc


class Base(metaclass=abc.ABCMeta):

    @abc.abstractmethod
    def test_method(self):
        raise NotImplementedError


class Child1(Base):

    def test_method(self):
        print('overloaded in Child1')


class Child2(Base):

    def no_test_method(self):
        print('another method')


c1 = Child1()
c2 = Child2()

Python behavior: TypeError: Can't instantiate ... Exception ✅ Pylint: W0223: Method 'test_method' is abstract ✅

A NotImplementedError is intended to be used for interfaces which are currently not implemented. So if some class tries to use (inherit) this method the NotImplementedError should be raised. If the method will be declared abstract the base method never will be called .

Warning

Implementation of NotImplementedError is wrong in Example 3, the next example shows the non-abstract (mixed) implementation using it with abc.ABCMeta, but not declared abstract (correct implementation).

Example 4 (NotImplementedError declared non-abstract)

import abc


class Base(metaclass=abc.ABCMeta):

    @abc.abstractmethod
    def some_abstract_method(self):
        print('i am abstract')

    def ni_method(self):
        raise NotImplementedError


class Child1(Base):

    def some_abstract_method(self):
        print('overloaded in Child1')


class Child2(Base):

    def some_abstract_method(self):
        print('overloaded in Child2')

c1 = Child1()
c2 = Child2()
c2.ni_method()

Python behavior: NotImplementedError Exception raised ✅ Pylint: 2x W0223: Method 'test_method' is abstract ❌

[clauspruefer]

@mbyrnepr2 Hi, is it possible to reference you / this conversation for a public discussion on https://der-it-pruefer.de?

[mbyrnepr2]

Yes feel free to do that. It’s very considerate of you to ask.