Skip to content

E704: Ignore def with "..." as its body as stub / @typing.overload #957

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

Closed

Conversation

terencehonles
Copy link

Handle a common pattern used by Python's @typing.overload

@asottile
Copy link
Member

asottile commented Aug 8, 2020

is this needed? E704 is disabled by default

@terencehonles
Copy link
Author

Would it be better to be a configured setting? It seems to be enabled by default in flake8, but I don't know if that's because an ignore list removes all the defaults. It doesn't seem to be a particularly expensive change and it is pretty strict about what it allows.

@asottile
Copy link
Member

asottile commented Aug 8, 2020

you probably want to use extend-ignore instead of ignore with flake8 fwiw

@terencehonles
Copy link
Author

terencehonles commented Aug 14, 2020

So, I'm guessing you'd rather not have this change? For this rule I'm probably more or less OK going with the default that I didn't realize I was overriding since the line length and other rules will probably keep the code from getting too hairy.

However, I was also contemplating on touching how many blank lines are required between @overrides since 2 blank lines between overrides for a function is really wordy (1 for a method is still not "great", but it's definitely better than 2). Before making that change, which is definitely more complex than this change, I'd want to know if I should even bother.

What I'd be wanting to change is to allow:

from typing import List, override, TypeVar

T = TypeVar('T')
U = TypeVar('U')


@override
def function(
        ...,
        paramN: T) -> T:
    ...
@override
def function(
        ...,
        paramN: U) -> List[T]:
    ...
def function(*args):
    ...

Instead of requiring:

from typing import List, override, TypeVar

T = TypeVar('T')
U = TypeVar('U')


@override
def function(
        ...,
        paramN: T) -> T:
    ...


@override
def function(
        ...,
        paramN: U) -> List[T]:
    ...


def function(*args):
    ...

The rationale for this change is that the further the definitions are apart from each other the less they are reading like they are related to each other. Since type definitions are both for the reader and the tool they should be as close to the function they are documenting. This change is related to that change because you may also want to allow a more terse @override:

from typing import override

@override
def function(value: str) -> str: ...
@override
def function(value: bytes) -> bytes: ...
def function(value):
    ...

@sigmavirus24
Copy link
Member

Closing and re-opening to trigger a new test build

@tobiasdiez
Copy link

It would be nice if this PR could be merged.

We have enabled E704 since in general that's a convenient choice (for us). However, in the context of overloaded methods its indeed overkill to consider ... to be a separate statement.

@Avasam
Copy link

Avasam commented Apr 8, 2022

You can even see it being used that way in:
https://peps.python.org/pep-0008/
https://peps.python.org/pep-0484/

And is the go-to format for type stubs

@asottile asottile closed this Mar 21, 2024
Joshix-1 added a commit to asozialesnetzwerk/an-website that referenced this pull request Mar 29, 2024
E704 is disabled by default ( PyCQA/pycodestyle#957 (comment) )
E704 conflicts with blacks new formatting ( PyCQA/pycodestyle#1227 )
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

Successfully merging this pull request may close these issues.

5 participants