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

Allow unreachable trailing yield, to make an abstract generator method #298

Open
Zac-HD opened this issue Jan 10, 2023 · 6 comments
Open

Comments

@Zac-HD
Copy link

Zac-HD commented Jan 10, 2023

Consider the following example, where an abstract method is a generator:

class AbstractFoo(ABC):
    @abstractmethod
    def some_generator(self):
        raise NotImplementedError
        yield

We want to raise NotImplementedError to prevent accidental calls, but then Vulture errors out because the yield is unreachable. Indeed it is unreachable - but we need the yield statement there to make the method into a generator!

Decent solutions might include supporting some way to ignore unreachable-code warnings (as is possible for unused variables), or more direct heuristics like "ignore an unreachable bare yield if there are no following statements and only a raise statement preceding" - I'm not attached to any particular solution, but would love something to unblock this so I can enforce Vulture rather than just running it by hand occasionally 🙂

@kreathon
Copy link
Contributor

kreathon commented Jan 13, 2023

Just randomly reading this.

but we need the yield statement there to make the method into a generator!

May I ask what the yield makes for a difference? So where does it break if it is missing? Runtime, type checking, other static checkers or just that the developer knows about it?

I am just asking such that I can think about other possible solutions.

@Zac-HD
Copy link
Author

Zac-HD commented Jan 13, 2023

Runtime (e.g. inspect.isgeneratorfunction() as well as static typechecking. I guess it's also useful for human readers but the runtime requirement is the hard one.

@kreathon
Copy link
Contributor

I agree that that type checker and code readability can probably easily be managed (type annotation, custom decorator, custom exception).

For runtime checks I thought about the following:

class AbstractFoo(ABC):
    @abstractmethod
    def some_generator(self):
        yield from GeneratorNotImplemented()

where GeneratorNotImplemented is something like

class GeneratorNotImplemented:
    def __init__(self):
        raise NotImplementedError

However, I guess it it less readable than the original code.

Sorry, if this goes into a wrong direction for you. I am not maintainer of this project.

Bye the way, the more I think about it, the more sense it makes to me to not mark them as unreachable yields (similar to yield followed by a return to create an empty iterator). Because the yield was put there on purpose and changes the behavior of the program.

I also checked mypy and PyCharm both of them will mark the code as unreachable (like vulture).

@sanmai-NL
Copy link

Don't raise NotImplementedError (class) but NotImplementedError() (instance).

@jakkdl
Copy link

jakkdl commented Jun 5, 2023

It's not documented in the README, but you can suppress unreachable-code errors for a line with noqa: V201. Vulture should perhaps handle this case automatically though.

@jfcherng
Copy link

jfcherng commented Jun 30, 2024

Here's another use case. I don't force a child class to implement the method and I have a default empty one.

    def my_generator(self) -> Generator[str, None, None]:
        return
        yield  # I have to add "noqa: V201"

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

5 participants