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

How to override a superclass method's docstring without triggering useless-super-delegation? #4228

Closed
jmehnle opened this issue Mar 11, 2021 · 4 comments
Labels

Comments

@jmehnle
Copy link

jmehnle commented Mar 11, 2021

I'm defining a small hierarchy of database adapters for MySQL and PostgreSQL. There's a base class that defines some common methods, and there's one subclass for each of MySQL and PostgreSQL. Each of these subclasses happen to override a particular method of the base class merely for the purpose of changing the docstring to explain what this method does in a MySQL vs. a PostgreSQL context, but the implementation doesn't change because the base class method delegates to private methods defined by the subclasses. Explaining the reasons for this design is outside the scope of this ticket, but please trust that there are good reasons.

class DatabaseAdapter(ABC):
    ...

    @abstractmethod
    def _getvar(self, name: str) -> Any:
        ...

    def getvar(self, name: str) -> Any:
        """Get the value of the given database variable."""
        return self._getvar(name)

class MySQLAdapter(DatabaseAdapter):
    ...

    def _getvar(self, name: str) -> Any:
        # Do the MySQL thing.
        ...

    def getvar(self, name: str) -> Any:
        """
        Get the value of the given database session variable.

        This is equivalent to `SELECT @@name`.
        """
        return super().getvar(name)

class PostgreSQLAdapter(DatabaseAdapter):
    ...

    def _getvar(self, name: str) -> Any:
        # Do the PostgreSQL thing.
        ...

    def getvar(self, name: str) -> Any:
        """
        Get the value of the given database run-time parameter.

        This is equivalent to `SHOW name`.
        """
        return super().getvar(name)

See how each class's getvar docstring differs? I want to maintain this difference in docstrings without changing the delegation behavior of the method.

However, this code makes pylint complain with a useless-super-delegation warning. I would argue that pylint should probably detect the subclass methods' different docstrings and take that as a sign that this is being done to override the base class method's docstring and not log a useless-super-delegation. Is there any way other than # pylint: disable=useless-super-delegation to resolve this?

@hippo91
Copy link
Contributor

hippo91 commented Apr 9, 2021

@jmehnle sorry for the delay. I do not think there is other way that disabling the useless-super-delegation message.
Maybe @Pierre-Sassoulas or @cdce8p will have better idea?

@Pierre-Sassoulas
Copy link
Member

If we let the docstrings being a little different not raise any message for this, then it will be considered a false negative when a bad copy paste is not detected. I think this is somewhat similar to #3536, we could maybe add a "super-delegation-only-redefine-docstring" message that can then be disabled project wide.

@cdce8p
Copy link
Member

cdce8p commented Apr 9, 2021

I tend to agree with @Pierre-Sassoulas here. pylint should focus on the code itself. It's too easy for docstrings to become outdated. Where I disagree is if it's really necessary to add a new option for it that subsequently needs to be maintained.

If it was my code, I would probably do # pylint: disable=useless-super-delegation ones at the beginning of the file and forget about it.

@hippo91
Copy link
Contributor

hippo91 commented Apr 18, 2021

Totally agree with @Pierre-Sassoulas and @cdce8p.

@hippo91 hippo91 closed this as completed Apr 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants