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 write a protocol that has a method that accepts arbitrary arguments? #2204

Closed
MapleCCC opened this issue Aug 18, 2021 · 7 comments
Closed
Labels
as designed Not a bug, working as intended

Comments

@MapleCCC
Copy link

I am not sure whether this is a bug or not. Did not find related information in PEP 544.

I would like to write a type protocol that has a method that accepts arbitrary arguments, yet Pyright complains about it.

from typing import Protocol
from typing_extensions import ParamSpec

P = ParamSpec("P")

class SupportsUndo(Protocol):
    def undo(self, *args, **kwargs) -> None:
        ...

class Task:
    def undo(self, aggressive: bool = False) -> None:
        pass

class Context:
    def undo(self, raise_exception: bool = True) -> None:
        pass

def rollback(something: SupportsUndo) -> None:
    ...

rollback(Task())
rollback(Context())

Pyright complains that Parameter "*args" has no corresponding parameter and Parameter "**kwargs" has no corresponding parameter. Should we consider this a bug? If not, what can we do to workaround this type checker complaint? Thanks in advance for any help.

@erictraut
Copy link
Collaborator

Pyright is doing the right thing here. A protocol defines an interface that must be honored by compatible classes. In your case, the protocol indicates that the undo method can take arbitrary *args and **kwargs, so it's legal to pass anything to it. The class Task and Context do not accept arbitrary arguments, and the program would crash if someone tried to call them with the wrong arguments.

As confirmation, mypy generates the same error.

It's fine to write a protocol with a method that accepts arbitrary arguments, but a class that is compatible with that protocol must also accept arbitrary arguments.

from typing import Protocol

class SupportsUndo(Protocol):
    def undo(self, *args, **kwargs) -> None:
        ...

class Task:
    def undo(self, *args, **kwargs) -> None:
        pass

def rollback(something: SupportsUndo) -> None:
    ...

rollback(Task())

@erictraut erictraut added the as designed Not a bug, working as intended label Aug 18, 2021
@MapleCCC
Copy link
Author

MapleCCC commented Aug 18, 2021

I understand the reasoning here. But if we annotate Task.undo() as accepting arbitrary argument, that dismatches run-time behavior when Task.undo() implementation actually only accepts an aggressive argument. Is there any way we can gap the discrepancy here?

@erictraut
Copy link
Collaborator

I guess I'm not clear on what you're trying to do. Will rollback call the undo method? If so, what arguments will it pass? If it will always pass zero arguments (and therefore aggressive and raise_exception will always receive their default values), then the protocol should indicate that undo does not accept any arguments.

class SupportsUndo(Protocol):
    def undo(self) -> None:
        ...

@MapleCCC
Copy link
Author

MapleCCC commented Aug 18, 2021

That's my bad. I used a bad example. Let me put a more detailed and make-sense example here:

I would like to write a small library that composes rollback-able task. It wraps user-defined class with __do__() and __undo__() methods into a context manager.

import contextlib
from collections.abc import Callable, Iterator
from contextlib import AbstractContextManager

def wrap_to_context_manager_function(cls: type[SupportsDoAndUndo]) -> Callable[..., AbstractContextManager[None]]:

    @contextlib.contextmanager
    def cmfunc(*args, **kwargs) -> Iterator[None]:

        task = cls()
        task.__do__(*args, **kwargs)

        try:
            yield

        except Exception:
            task.__undo__()
            raise

    return cmfunc

Now user can write their own class like this:

from pathlib import Path

class AtomicWrite:
    def __do__(self, file: Path, text: str) -> None:
        self.file = file
        self.old_text = file.read_text()
        file.write_text(text)

    def __undo__(self) -> None:
        self.file.write_text(self.old_text)

And then my library helps to wrap the user-defined class into an easy-to-use context manager function:

atomic_write = wrap_to_context_manager_function(AtomicWrite)

with atomic_write("output.txt", "some text"):
    # Some thing that may fail and trigger rollback which restores the content of output.txt

Hope this make more sense now. Anyway, we need to have a protocol SupportsDoAndUndo:

class SupportsDoAndUndo(Protocol):
    def __do__(self, *args, **kwargs) -> None:
        ...

    def __undo__(self) -> None:
        ...

This goes back to where our original discussion is.

@erictraut
Copy link
Collaborator

OK, I understand what you're trying to do now. A protocol isn't a great solution in this circumstance because you can't describe the interface you're trying to define.

The best suggestion I can think of is:

class SupportsDoAndUndo(Protocol):
    __do__: Callable[..., None]
    __undo__: Callable[[], None]

This type checks fine in pyright, but mypy doesn't accept it due to a known bug in mypy that hasn't yet been fixed.

@JelleZijlstra
Copy link
Contributor

See python/mypy#5876 for some related discussion.

@MapleCCC
Copy link
Author

@erictraut Thanks for the suggestion. The workaround works well for my case. I will settle with this workaround for now. Sorry for spending your time to read my lengthy example. I will try reduce it to a more minimal example next time.

@JelleZijlstra Thanks for the link. That's exactly the issue I am encountering. Subscribed it for future update.

Also, should we maybe update this issue's title to be a more accurate and informative one, so that others who encounter similar issue in the future can benefit? I feel like the current title is not being very helpful for searching, but I am also not super familiar with the type system terminology to make it more accurate though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as designed Not a bug, working as intended
Projects
None yet
Development

No branches or pull requests

3 participants