Use WalletProtocol to constrain wallet function type signatures

[aqk]

Code changes - no user visible changes

[xdustinface]

There are some reasons in @altendky's mind why we shouldn't inherit from a Protocol (Something because its a metaclass and thats bad for some reason). I can't tell you more about that but what i can tell is that we have

chia-blockchain/chia/pools/pool_wallet.py

Lines 993 to 996 in 2c25a79

	if TYPE_CHECKING:
	from chia.wallet.wallet_protocol import WalletProtocol
	_dummy: WalletProtocol = cast(PoolWallet, None)

in each separate wallet file which already takes care of forcing the wallets to follow the WalletProtocol. I guess thats what you wanted to archive with this PR?

[altendky]

As I recall, inheriting from a Protocol is making another Protocol. Also, yes, given the opportunity to not inherit from a metaclass, I do (not). A given class can only have one metaclass. As such, using one is a promise to never use another (at the same time) which makes it an extremely limiting pattern. They are also not a thing that people are generally going to be familiar with. A metaclass redefines not just how the instances work, but how the class itself works. I think Protocol itself doesn't do anything weird, but using metaclasses is treading into hairy territory.

I did have a decorator that could be used to check stuff like this, but mypy doesn't presently output the detailed info that would be more helpful with debugging when using the hint checking mechanism the decorator does. I've got a PR at python/mypy#13878 where I had started to work on that. Needs tests though, if I remember correctly. It does have an example of the decorator tooling. The primary upside of the decorator is that it keeps the expression of following the protocol in the area that people expect to see it. Right new the class keyword usage.

[altendky]

Hmm, so if we use quotes with cast() we can actually move the existing checks up where we would prefer them. That seems like a worthwhile improvement to legibility here.

https://mypy-play.net/?mypy=1.1.1&python=3.11&gist=4456b7f7f773465169b70c3113082311

from __future__ import annotations

from typing import TYPE_CHECKING, Protocol, cast


class P(Protocol):
    def m(self) -> int:
        ...


if TYPE_CHECKING:
    __protocol_check: P = cast("C", None)

class C:
    def m(self) -> str:
        return ""

main.py:12: error: Incompatible types in assignment (expression has type "C", variable has type "P")  [assignment]
main.py:12: note: Following member(s) of "C" have conflicts:
main.py:12: note:     Expected:
main.py:12: note:         def m(self) -> int
main.py:12: note:     Got:
main.py:12: note:         def m(self) -> str
Found 1 error in 1 file (checked 1 source file)

Or maybe better yet, right inside the class.

class C:
    if TYPE_CHECKING:
        _protocol_check: P = cast("C", None)

    def m(self) -> str:
        return ""

[altendky]

Bother, I keep fiddling a little more and realizing that I missed all this in my first exploration. No need for quotes inside the class.

class C:
    if TYPE_CHECKING:
        _protocol_check: P = cast(C, None)

    def m(self) -> str:
        return ""

This is brief, well located, doesn't pollute the global namespace since _protocol_check is a class attribute, is obviously playing type checking games. The two main downsides in my mind are that it is a pattern with no centralized structure around which to describe the pattern (the decorator would have this) and that it does introduce an attribute on the class that people might misread and try to use. The latter can be dealt with with a name. I guess the former could be dealt with with a pinned link to the PGP doc. If we like this I can do a little write up for this pattern in the doc.

[altendky]

Maybe #15134 is a satisfactory replacement?

[arvidn]

to repeat what kyle said. This is not how you declare that you implement a protocol, this is how you declare a new protocol that subsumes another.

My understanding is that whether a type implements a protocol is essentially duck-typing in mypy

[altendky]

also known as structural subtyping, in case you see that floating around. i still intend to get a smaller pattern working someday but at the moment mypy doesn't provide verbose output clarifying in what way the protocol is not satisfied when you do it my 'better' way.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties.