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

Fix isinstance check for Generic classes #188

Merged
merged 2 commits into from
May 24, 2023
Merged

Fix isinstance check for Generic classes #188

merged 2 commits into from
May 24, 2023

Conversation

dolfinus
Copy link
Contributor

@dolfinus dolfinus commented May 24, 2023

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

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented May 24, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

Copy link
Member

@AlexWaygood AlexWaygood left a comment

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)

src/typing_extensions.py Show resolved Hide resolved
@AlexWaygood
Copy link
Member

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

@dolfinus
Copy link
Contributor Author

Added test and changelog item

CHANGELOG.md Outdated Show resolved Hide resolved
@AlexWaygood
Copy link
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
Copy link
Member

@AlexWaygood AlexWaygood left a comment

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
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants