pep8-naming icon indicating copy to clipboard operation
pep8-naming copied to clipboard

`N803`, `N804` and `N805` should also ignore methods marked with `@override`

Open ivanlonel opened this issue 1 year ago • 1 comments

As pointed out in #215 and #217, overridden method names are out of the dev's control when subclassing an external library.

But that actually extends to the whole method signature. Renaming a method parameter in a subclass breaks LSP, so I believe ignoring it in these cases would avoid inducing the user to a bad practice.

Example using QGIS Python API:

class MyFeedback(QgsProcessingFeedback):
    @override
    def reportError(self, error: str, fatalError: bool = False) -> None:  # noqa: N803
        if fatalError:
            logger.critical(error)
        else:
            logger.error(error)

fatalError is not positional-only, so renaming it to fatal_error would cause an error if the method was called as feedback.reportError("oh no", fatalError=True) somewhere else.

ivanlonel avatar May 17 '24 02:05 ivanlonel

On second thought, maybe ignore N803 but leave N804/N805 as they are?

Considering a first parameter not called self/cls seems less likely on third-party libraries, it could actually just be a mistake on the user's side.

On the rare occasion it's actually necessary to ignore these warnings, it seems OK to just #noqa them, which would also signal to a reader who knows pep8-naming: "hey, see that odd-named first parameter? It should be self/cls, but there's nothing I can do about it because I don't own the base class".

ivanlonel avatar May 17 '24 16:05 ivanlonel