Skip to content

Fix isinstance check for Generic classes - #188

Merged
srittau merged 2 commits into
python:mainfrom
dolfinus:main
May 24, 2023
Merged

Fix isinstance check for Generic classes#188
srittau merged 2 commits into
python:mainfrom
dolfinus:main

Conversation

@dolfinus

@dolfinus dolfinus commented May 24, 2023

Copy link
Copy Markdown
Contributor

After upgrading to typing-extensions==4.6.1 this code example started failing on Python 3.7:

from typing_extensions import Protocol, runtime_checkable
from typing import Generic, TypeVar
from pathlib import Path
from dataclasses import dataclass

@runtime_checkable
class ContainsException(Protocol):
    """
    Protocol for objects containing ``.exception`` attribute
    """

    @property
    def exception(self) -> Exception:
        """
        Exception object with traceback
        """

T = TypeVar("T")

@dataclass(eq=False, frozen=True)
class PathContainer(Generic[T]):
    path: T
    exception: Exception

@dataclass(eq=False, frozen=True)
class PathWithFailure(PathContainer[Path]):
    pass


file = PathWithFailure(path=Path('/some'), exception=RuntimeError("some"))
isinstance(file, ContainsException)
Traceback (most recent call last):
  File "/home/maxim/Repo/typing_extensions/1.py", line 33, in <module>
    isinstance(file, ContainsException)
  File "/home/maxim/Repo/typing_extensions/src/typing_extensions.py", line 599, in __instancecheck__
    if super().__instancecheck__(instance):
  File "/home/maxim/.pyenv/versions/3.7.8/lib/python3.7/abc.py", line 139, in __instancecheck__
    return _abc_instancecheck(cls, instance)
  File "/home/maxim/Repo/typing_extensions/src/typing_extensions.py", line 583, in __subclasscheck__
    return super().__subclasscheck__(other)
  File "/home/maxim/.pyenv/versions/3.7.8/lib/python3.7/abc.py", line 143, in __subclasscheck__
    return _abc_subclasscheck(cls, subclass)
  File "/home/maxim/Repo/typing_extensions/src/typing_extensions.py", line 661, in _proto_hook
    and other._is_protocol
AttributeError: type object 'PathWithFailure' has no attribute '_is_protocol'

Who said that objects of generic class must have _is_protocol property?

Caused by #184

@ghost

ghost commented May 24, 2023

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I think this probably also needs a changelog entry (under a new "Unreleased" heading at the top of the file)

Comment thread src/typing_extensions.py
@AlexWaygood

Copy link
Copy Markdown
Member

(A test would also be great, to ensure that this doesn't regress again)

@dolfinus

Copy link
Copy Markdown
Contributor Author

Added test and changelog item

Comment thread CHANGELOG.md Outdated
Comment thread src/test_typing_extensions.py Outdated
@AlexWaygood

Copy link
Copy Markdown
Member

The following commit authors need to sign the Contributor License Agreement:

Click the button to sign:CLA not signed

Looks like your latest commit might have been authored using a different email address, which is upsetting the CLA bot. Would you mind signing it again with your second email address?

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@dolfinus
dolfinus requested a review from AlexWaygood May 24, 2023 10:05

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I confirmed that the new test fails on main, and passes with this PR. Sorry again for the breakage!

@srittau
srittau merged commit 57aae62 into python:main May 24, 2023
@JelleZijlstra

Copy link
Copy Markdown
Member

This fix was released as part of version 4.6.2 just now.

@AlexWaygood AlexWaygood mentioned this pull request May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants