python-attrs / cattrs

Composable custom class converters for attrs, dataclasses and friends.
https://catt.rs
MIT License
799 stars 111 forks source link

Simplify `cattrs._compat.is_typeddict` #384

Closed AlexWaygood closed 1 year ago

AlexWaygood commented 1 year ago

Following up from https://github.com/python/typing_extensions/issues/230#issuecomment-1587750089! Not sure why the test for generic typeddicts on 3.11 is failing.

This:

AlexWaygood commented 1 year ago

Hmm, the test suite is succeeding on PyPy but the "upload to codecov" bit is failing, and then that leads to all other jobs being cancelled

codecov-commenter commented 1 year ago

Codecov Report

Merging #384 (97774e2) into main (05175ef) will decrease coverage by 0.01%. The diff coverage is 72.72%.

:exclamation: Current head 97774e2 differs from pull request most recent head 019bc63. Consider uploading reports for the commit 019bc63 to get more accurate results

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main     #384      +/-   ##
==========================================
- Coverage   95.75%   95.75%   -0.01%     
==========================================
  Files          26       26              
  Lines        2122     2121       -1     
==========================================
- Hits         2032     2031       -1     
  Misses         90       90              
Impacted Files Coverage Δ
src/cattrs/_compat.py 95.45% <72.72%> (-0.48%) :arrow_down:

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

Tinche commented 1 year ago

Wow Codecov is really being garbage today, sorry about that heh.

Tinche commented 1 year ago

Here's a reduced version of the failing test case:

from typing import TypedDict, Generic, TypeVar
from cattrs._compat import is_typeddict

T = TypeVar("T")

class A(TypedDict, Generic[T], total=True):
    a: T

print(is_typeddict(A[int]))  # False, should be true
AlexWaygood commented 1 year ago

Here's a reduced version of the failing test case:

Ahh, thank you! Should be fixed now -- all tests are now passing for me locally :)

AlexWaygood commented 1 year ago

Also, I should have spotted that behaviour difference between cattrs's version of is_typeddict and the typing(_extensions) one just by looking at the code 🤦

Tinche commented 1 year ago

LGTM, <3 can you just add a line to the changelog? Doesn't have to be very specific, but might help someone down the line if there's a behavior change we're not testing for.

AlexWaygood commented 1 year ago

LGTM, <3 can you just add a line to the changelog? Doesn't have to be very specific, but might help someone down the line if there's a behavior change we're not testing for.

Done! Lmk if that's not quite what you were looking for :)

Tinche commented 1 year ago

Great, thanks!