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

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

Open
ivanlonel opened this issue May 17, 2024 · 1 comment
Open

Comments

@ivanlonel
Copy link

ivanlonel commented May 17, 2024

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
Copy link
Author

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".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant