Pylint: Useless super delegation in method when overriding abstract methods.

Created on 26 Jul 2017  Â·  4Comments  Â·  Source: PyCQA/pylint

from contextlib import AbstractContextManager

class ProgramContext(AbstractContextManager):

    # Overridden methods ------------------------------------------------------
    # Overrides method from AbstractContextManager.
    def __exit__(self, exc_type, exc_value, traceback):
        super().__exit__(exc_type, exc_value, traceback)

This gives "useless super delegation", but this method implements an abstract method from AbstractContextManager. The super delegation is required if this class is to participate in a polymorphic hierarchy since it doesn't know where in the inheritance chain it will be inherited.

Most helpful comment

@PCManticore

I think we can both agree that:

  • it is impossible to inherit form AbstractContextManager without overriding this method,
  • super has to be called in case there are other mixins in the inheritance hierarchy, and
  • it is sometimes desirable to define a context manager that does nothing in its exit.

Therefore, this is reasonable pattern that should not give an error.

The solution is to not flag this error when the parent method is marked abstract, which can easily be checked using if getattr(method, "__isabstractmethod__", False)} on the superclass.

Also, as you say:

It actually works because the original __exit__ method doesn't do anything and calling an abstract method doesn't raise any TypeError whatsoever.

This is not required. First of all, usually abstract methods raise NotImplementedError — not TypeError. And, even so, they do not have to raise anything. In fact, AbstractContextManager is in the standard library, which is proof that abstract methods can have implementations. I don't like that they do, but this is the direction that Python has taken, and we are all stuck with it.

All 4 comments

I'm not sure this makes sense. Yes, the original method is abstract and needs to be implemented, but in this case the implementation is moot, since in doesn't do anything but delegating to an empty implementation of in the base class. It actually works because the original __exit__ method doesn't do anything and calling an abstract method doesn't raise any TypeError whatsoever.

It's not moot. Without this method definition, the class cannot be instantiated.

Please disable the warning locally, this is not a bug in pylint.

@PCManticore

I think we can both agree that:

  • it is impossible to inherit form AbstractContextManager without overriding this method,
  • super has to be called in case there are other mixins in the inheritance hierarchy, and
  • it is sometimes desirable to define a context manager that does nothing in its exit.

Therefore, this is reasonable pattern that should not give an error.

The solution is to not flag this error when the parent method is marked abstract, which can easily be checked using if getattr(method, "__isabstractmethod__", False)} on the superclass.

Also, as you say:

It actually works because the original __exit__ method doesn't do anything and calling an abstract method doesn't raise any TypeError whatsoever.

This is not required. First of all, usually abstract methods raise NotImplementedError — not TypeError. And, even so, they do not have to raise anything. In fact, AbstractContextManager is in the standard library, which is proof that abstract methods can have implementations. I don't like that they do, but this is the direction that Python has taken, and we are all stuck with it.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

PCManticore picture PCManticore  Â·  3Comments

adamtheturtle picture adamtheturtle  Â·  3Comments

DevynCJohnson picture DevynCJohnson  Â·  3Comments

sambarluc picture sambarluc  Â·  3Comments

mrginglymus picture mrginglymus  Â·  3Comments