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

Ruh-roh! CACHING protocols? More like CRASHING protocols (in 3.7)! Amirite? #109

Closed
posita opened this issue Mar 5, 2022 · 12 comments
Closed
Assignees

Comments

@posita
Copy link
Collaborator

posita commented Mar 5, 2022

So here's a head-scratcher:

# test_ruh_roh.py
from abc import abstractmethod
from beartype import beartype
from beartype.typing import TYPE_CHECKING, Iterable, Protocol, TypeVar, Union, runtime_checkable

_T_co = TypeVar("_T_co", covariant=True)

if TYPE_CHECKING:
    from abc import ABCMeta as _BeartypeCachingProtocolMeta
else:
    _BeartypeCachingProtocolMeta = type(Protocol)

# - - - - - - - - - - - - - - - - SupportsInt - - - - - - - - - - - - - - - -

@runtime_checkable
class SupportsInt(
    Protocol,  # <-- this is totally cool
    metaclass=_BeartypeCachingProtocolMeta,
):
    __slots__: Union[str, Iterable[str]] = ()
    @abstractmethod
    def __int__(self) -> int:
        pass

@beartype  # <-- this works
def myint(arg: SupportsInt) -> int:
    return int(arg)

print(f"{myint(0.0)} == {0}?")
assert myint(0.0) == 0

# - - - - - - - - - - - - - - - - SupportsAbs[_T_co] - - - - - - - - - - - - - - - -

@runtime_checkable
class SupportsAbs(
    Protocol[_T_co],  # <-- uh-oh, now we're askin' for trouble
    metaclass=_BeartypeCachingProtocolMeta,
):
    __slots__: Union[str, Iterable[str]] = ()
    @abstractmethod
    def __abs__(self) -> _T_co:
        pass

@beartype  # <-- boom
def myabs(arg: SupportsAbs[_T_co]) -> _T_co:
    return abs(arg)

print(f"{myabs(-1)} == {1}?")
assert myabs(-1) == 1

Using beartype @ 8da3ab4 in 3.8:

% python3.8 -m mypy test_ruh_roh.py
Success: no issues found in 1 source file
% python3.8 test_ruh_roh.py
0 == 0?
1 == 1?

And now 3.7:

% python3.7 -m mypy test_ruh_roh.py
Success: no issues found in 1 source file
% python3.7 test_ruh_roh.py
0 == 0?
Traceback (most recent call last):
  File "test_ruh_roh.py", line 45, in <module>
    def myabs(arg: SupportsAbs[_T_co]) -> _T_co:
  File "/…/beartype/beartype/_decor/main.py", line 193, in beartype
    return beartype_args_mandatory(obj, conf)
  File "/…/beartype/beartype/_decor/_core.py", line 140, in beartype_args_mandatory
    return _beartype_func(func=obj, conf=conf)
  File "/…/beartype/beartype/_decor/_core.py", line 171, in _beartype_func
    func_wrapper_code = generate_code(func_data)
  File "/…/beartype/beartype/_decor/_code/codemain.py", line 222, in generate_code
    code_check_params = _code_check_args(bear_call)
  File "/…/beartype/beartype/_decor/_code/codemain.py", line 477, in _code_check_args
    func=bear_call.func_wrappee, arg_name=arg_name),
  File "/…/beartype/beartype/_util/error/utilerror.py", line 202, in reraise_exception_placeholder
    raise exception.with_traceback(exception.__traceback__)
  File "/…/beartype/beartype/_decor/_code/codemain.py", line 392, in _code_check_args
    exception_prefix=EXCEPTION_PREFIX,
  File "/…/beartype/beartype/_util/hint/utilhintconv.py", line 214, in sanify_hint_root
    die_unless_hint(hint=hint, exception_prefix=exception_prefix)
  File "/…/beartype/beartype/_util/hint/utilhinttest.py", line 102, in die_unless_hint
    die_unless_hint_nonpep(hint=hint, exception_prefix=exception_prefix)
  File "/…/beartype/beartype/_util/hint/nonpep/utilnonpeptest.py", line 206, in die_unless_hint_nonpep
    f'{exception_prefix}type hint {repr(hint)} either '
beartype.roar.BeartypeDecorHintNonpepException: @beartyped myabs() parameter "arg" type hint __main__.SupportsAbs[+_T_co] either PEP-noncompliant or currently unsupported by @beartype.

I'm scratching my head for two reasons: 1) numerary gets away with something nearly identical without breaking, even in 3.7; and 2) everything works as expected for 3.8.

So I think I screwed up a translation somewhere (most likely in #103).

@posita posita self-assigned this Mar 5, 2022
posita added a commit to posita/beartype that referenced this issue Mar 5, 2022
posita added a commit to posita/beartype that referenced this issue Mar 5, 2022
@leycec
Copy link
Member

leycec commented Mar 5, 2022

So. It comes to this. Tests are currently passing but they shouldn't be.

ruh roh

@leycec
Copy link
Member

leycec commented Mar 5, 2022

Okay. We're gonna dig deep into the fetid bowels of the beartype codebase. Strap on your corpse-digging gloves, boys!

The above exception means that beartype failed to recognize SupportsAbs[_T_co] as a valid type hint, right? So how does beartype recognize type hints, anyway? I'm so glad you asked. The answer to this question and more lies rotting in the low-level beartype._util.hint.pep.utilpepget.get_hint_pep_sign_or_none() getter. This getter is the beating heart of @beartype. It detects whether the passed object is a valid type hint and if that object is:

A few pithy examples, because I finally got to use the word "pithy": slam dunk

>>> from beartype._util.hint.pep.utilpepget import get_hint_pep_sign_or_none
>>> get_hint_pep_sign_or_none(list[str])
HintSignList
>>> get_hint_pep_sign_or_none(str | int)
HintSignUnion
>>> get_hint_pep_sign_or_none(str)
None

Simple types have no signs. Everything else supported by beartype does. Now, we just need to dig into the beartype._util.hint.pep.utilpepget.get_hint_pep_sign_or_none() getter to figure out why this is failing for fast subscripted protocols under Python 3.7. Curse this Friday evening! 🤬

@leycec
Copy link
Member

leycec commented Mar 5, 2022

Also! This issue smells suspiciously like this conditional in _typingpep544 is failing to behave as expected under Python 3.7:

                # Protocol class to be passed as the "cls" parameter to the
                # unwrapped superclass typing.Protocol.__class_getitem__()
                # dunder method. There exist two unique cases corresponding to
                # two unique branches of an "if" conditional in that method,
                # depending on whether either this "Protocol" superclass or a
                # user-defined subclass of this superclass is being
                # subscripted. Specifically, this class is...
                protocol_cls = (
                    # If this "Protocol" superclass is being directly
                    # subclassed by one or more type variables (e.g.,
                    # "Protocol[S, T]"), the non-caching "typing.Protocol"
                    # superclass underlying this caching protocol superclass.
                    # Since the aforementioned "if" conditional performs an
                    # explicit object identity test for the "typing.Protocol"
                    # superclass, we *MUST* pass that rather than this
                    # superclass to trigger that conditional appropriately.
                    _ProtocolSlow
                    if cls is Protocol else
                    # Else, a user-defined subclass of this "Protocol"
                    # superclass is being subclassed by one or more type
                    # variables *OR* types satisfying the type variables
                    # subscripting the superclass (e.g.,
                    # "UserDefinedProtocol[str]" for a user-defined subclass
                    # class UserDefinedProtocol(Protocol[AnyStr]). In this
                    # case, this subclass as is.
                    cls
                )

It's possible that the _Protocol.__class_getitem__() implementation under Python 3.7 differs substantially from that of Protocol.__class_getitem__() under Python ≥ 3.8. Then again, I desperately crave sleep but have yet to achieve it. Nothing a sleepless insomniac somnambulist says should be trusted. 😮‍💨

@posita
Copy link
Collaborator Author

posita commented Mar 5, 2022

Oh no! 😬😢

I did not mean to ruin your Friday night. Those should be reserved for date night, or sipping hot tea by the wood pellet stove with a good book, or sipping luke warm tea from a thermos while freezing your butt off on some aluminum bleachers attending your kid's (or friend's kid's or other junior relative's or acquaintance's, but you know, like in a supportive way, not a super creepy way) extracurricular event, or feverishly trying to "just" finish that last minute herculean project for the boss who stopped by your desk at 4:30 p.m. before leaving you with a wink and a finger gun on his way out to "hit the slopes" this weekend for some "R&R". Wait. No. Scratch that last one. That doesn't belong.

FWIW, I think you're onto something with _Protocol.__class_getitem__(). That was definitely absent in numerary's original approach. I tried to sloppily and temporarily replace the implementation with numerary's old braindead version as a way to validate the theory, but didn't get very far (yet).

<unhelpfulrant>As an aside, after going back and re-reading some of the tortured work-arounds we've had to engage in, I have to wonder whether things really should be this hard. My sense is, "no", but that is something outside of my control. 😞 Sigh.</unhelpfulrant>

@leycec
Copy link
Member

leycec commented Mar 8, 2022

date night

That's a funny way to spell "Mandatory Friday Afternoons at the Rick & Morty Matinee." Cottage country: you know we gets wild here. 😏

sipping hot tea by the wood pellet stove with a good book

You know us well. Our 500-pound Main Coon cat Wolfie (so-named for good reason) rumbles contentedly. Your knowledge of the hidden life and hobbies of the average bottom-feeding Canadian cabin-dweller is... uncanny.

I intended to insert additional quips here, but then got tired. Instead:

  • I merged [WIP] Failing test for #109 #110 to expose the rot at the core of our beartype.typing.Protocol Python 3.7 backport. We rejoice in our shared failures.
  • I wonder if this logic might suffice to fix this. Despite being untested in every way, I choose to believe in hope and change. I literally just added or IS_PYTHON_3_7, so there's no way this works:
                # Protocol class to be passed as the "cls" parameter to the
                # unwrapped superclass typing.Protocol.__class_getitem__()
                # dunder method. There exist two unique cases corresponding to
                # two unique branches of an "if" conditional in that method,
                # depending on whether either this "Protocol" superclass or a
                # user-defined subclass of this superclass is being
                # subscripted. Specifically, this class is...
                protocol_cls = (
                    # If this "Protocol" superclass is being directly
                    # subclassed by one or more type variables (e.g.,
                    # "Protocol[S, T]"), the non-caching "typing.Protocol"
                    # superclass underlying this caching protocol superclass.
                    # Since the aforementioned "if" conditional performs an
                    # explicit object identity test for the "typing.Protocol"
                    # superclass, we *MUST* pass that rather than this
                    # superclass to trigger that conditional appropriately.
                    _ProtocolSlow
                    if cls is Protocol or IS_PYTHON_3_7 else
                    # Else, a user-defined subclass of this "Protocol"
                    # superclass is being subclassed by one or more type
                    # variables *OR* types satisfying the type variables
                    # subscripting the superclass (e.g.,
                    # "UserDefinedProtocol[str]" for a user-defined subclass
                    # class UserDefinedProtocol(Protocol[AnyStr]). In this
                    # case, this subclass as is.
                    cls
                )

Mr. Hypno Palm

🦾

...I have to wonder whether things really should be this hard.

...heh. If it wasn't hard, Guido would have done it. This is why he didn't. We suffer that others may actually use protocols at runtime. We do this in remembrance of typing.Protocol, that no one may ever use that again.

leycec added a commit that referenced this issue Mar 9, 2022
This commit is the next in a commit chain backporting PEP 544 (i.e., the
`typing.Protocol` superclass) to Python 3.7 via our custom
`beartype.typing.Protocol` superclass, driven under the hood by the
third-party `typing_extensions.Protocol` superclass. Specifically, this
commit streamlines unit tests exercising this functionality. Naturally,
issue #109 remains unresolved. (*Streaming dreamlike beams like unhikeable seams!*)
@leycec
Copy link
Member

leycec commented Mar 9, 2022

lolbro. Against all semblance of sanity, if cls is Protocol or IS_PYTHON_3_7 else actually solves everything under Python 3.7. Assuming tox passes, hard stare we are immediately:

  • Pushing this nonsensical resolution.
  • Closing this sanity-destroying issue.
  • Publishing beartype 0.10.3 containing this resolution.

chug! chug!
We chugging this jug.

@leycec
Copy link
Member

leycec commented Mar 9, 2022

Woah. This is why we are not doing this again. Right? Right?

Basically, typing_extensions doesn't behave at all like typing at runtime. This means that @beartype generally wants to avoid typing_extensions at all costs, because supporting typing_extensions requires a reenactment of the ending of Oedipus Rex. In this case, the edge case exposed by this issue is actually super non-trivial to support. Of course, that means we're supporting it.

Still, let's try to avoid typing_extensions in the future where at all feasible. Based on my late-night ruminations on the dark precipice of sanity, this edge case actually never worked as intended – including within numerary. The core issue is that:

  • typing_extension protocols only subclass Protocol.
  • typing protocols subclass both Protocol and Generic.

Surprisingly, this matters. @beartype internally detects subscripted protocols as subclassing Generic. Ergo, this edge case fails when using typing_extension protocols. 🤦 🤦‍♂️ 🤦‍♀️

@leycec
Copy link
Member

leycec commented Mar 9, 2022

My eye is twitching, bro. The only sane way to resolve this is to monkey-patch the hideous 500-line typing_extensions.Protocol.__init_subclass__() method with one of our own demented devising that behaves less demented.

Arise, demonic forces! Take this issue straight to the Seventh Circle!

@leycec
Copy link
Member

leycec commented Mar 9, 2022

Actually... I feel like a complete jerk, but I'm afraid this ain't gonna work. I ended up copy-and-pasting the entire friggin' Python 3.7-specific typing_extensions.Protocol class definition into our beartype.typing._typingpep544 submodule – and it's still broken af.

I'm kinda unsure anyone even cares about protocols under Python 3.7 besides us mice, either. Since typing_extensions.Protocol is busted at runtime and can't be cleanly repaired, this is clearly the wrong approach.

With extreme apologies, I'm gonna judiciously rip out our caching beartype.typing.Protocol backport. Some things simply can't be done. This is such a thing. So much "Ugh!" right now.

Wouldn't blame if you feel hyper frustrated with some combination of me, typing, typing_extensions, or just typing in general. I do. Let's execute this rotting code with extreme prejudice, @beartype. </sigh>

leycec added a commit that referenced this issue Mar 10, 2022
This commit is the last in a commit chain reverting our sadly failed
attempt to backport PEP 544 (i.e., the `typing.Protocol` superclass) to
Python 3.7 via our custom `beartype.typing.Protocol` superclass, driven
under the hood by the third-party `typing_extensions.Protocol`
superclass. Since the third-party `typing_extensions.Protocol`
superclass both fails to subclass `typing.Generic` *and* prohibits all
subclasses from doing so, that superclass is not merely
PEP 544-noncompliant; it's anti-PEP 544-compliant, because it actively
obstructs downstream consumers from being PEP 544-compliant.
Specifically, this commit gracefully undoes all of the prior commits of
this chain to to preserve maintainability, portability, and usability --
technically "resolving" issue #109 (kindly submitted by ace typing
expert @posita) in the worst way possible. Blame `typing_extensions`,
for it deserves heckling and hurled peanuts. (*Subdued subnautical dues!*)
@leycec
Copy link
Member

leycec commented Mar 10, 2022

it happened

So. Our beartype.typing.Protocol backport to Python 3.7 failed hideously and we had to surgically remove the festering rot before it multiplied like in that scifi movie you're trying really hard not to remember right now. 👽

So. Here's what happened. What happened is that typing_extensions is mostly untrustworthy and should be avoided wherever feasible. Specifically, the third-party typing_extensions.Protocol superclass both fails to subclass typing.Generic and prohibits all subclasses from doing so. This means that superclass is not merely PEP 544-noncompliant; it's anti-PEP 544-compliant, which is another tier of badness. typing_extensions.Protocol actively obstructs downstream consumers from being PEP 544-compliant with this eldritch code construct:

    # Note the lack of all mention of "typing.Generic" here.
    class Protocol(metaclass=_ProtocolMeta):
        ...

        def __init_subclass__(cls, *args, **kwargs):
            tvars = []

            # *THIS IS INSANITY INCARNATE.* typing_extensions,
            # why are you actively lobbing rocket-propelled
            # grenades into our attempted workflow?
            if '__orig_bases__' in cls.__dict__:
                error = typing.Generic in cls.__orig_bases__
            else:
                error = typing.Generic in cls.__bases__
            if error:
                raise TypeError("Cannot inherit from plain Generic")
            ...

If typing_extensions.Protocol only failed to subclass typing.Generic, we could have trivially circumvented that by just subclassing our own beartype.typing.Protocol from typing.Generic. For unknown and presumably indefensible reasons, typing_extensions.Protocol prevents us from doing so.

Altogether, these two Invisible Walls of Force mean that typing_extensions.Protocol has fallen and cannot get up. We can't subclass around it. We can't monkey-patch it. I tried. We can only ignore it and define our custom _ProtocolSlow implementation under Python 3.7. But I tried that, too. Everything failed.

At 4:00 last night, I began asking myself why we were even doing this. And I realized, @posita, that I myself no longer knew. No one really cares about protocols under Python 3.7, because no one really cares about Python 3.7, because Python 3.7 is on the precipice of hitting its own End-of-Life (EOL) and becoming a raging security concern.

Let's agree to never speak of this black day again. Black hole sun: rise from the depths and consume the third-party packages that assail us, we bid thee! ⚫

tl;dr

typing_extensions backports are mostly not backports. They're pre-release preview alpha draft implementations that are mostly broken at runtime.

We support typing_extensions.Annotated only because we have to and because it was actually feasible to do so. typing_extensions.Protocol drops both of those balls.

@posita
Copy link
Collaborator Author

posita commented Mar 11, 2022

Thank you for removing the festering nonsense that I introduced to pollute an otherwise pristine codebase! My apologies for not doing it myself sooner. 😞

If I didn't already tell you, one of my favorite things is to delete problematic code. I'm chalking this up to a win. ❤️

Anyone can write code. It takes a real bear to delete it.

@leycec
Copy link
Member

leycec commented Mar 11, 2022

As a fellow festering nonsense aficionado, I actually loved your festering nonsense. It spoke to me. The burden of shame is entirely on typing_extensions.Protocol, which violates PEP 544 and thus basic sanity. That class isn't even a valid protocol backport. I don't even... yet I do.

My gears are being grinded, typing_extensions. You're on the short bus straight to our short list! 🚌

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

2 participants