-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
Simplify cattrs._compat.is_typeddict
#384
Conversation
src/cattrs/_compat.py
Outdated
@@ -20,15 +20,18 @@ | |||
from attr import fields as attrs_fields | |||
from attr import resolve_types | |||
|
|||
__all__ = ["ExtensionsTypedDict", "is_typeddict", "TypedDict"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like __all__
was deleted from this file in #382, but I'm not entirely sure why. I've added it back here, as otherwise ruff complains (reasonably) that is_typeddict
is imported but unused. The alternative would be to do from typing_extensions import is_typeddict as is_typeddict
, but I find that syntax really ugly personally :p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whoops, my bad. You might wanna add ExceptionGroup
too which uses the x as x
syntax currently.
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 Report
❗ 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
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Wow Codecov is really being garbage today, sorry about that heh. |
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 |
Ahh, thank you! Should be fixed now -- all tests are now passing for me locally :) |
Also, I should have spotted that behaviour difference between cattrs's version of |
This will make it easier to see in python-attrs#384 which tests (if any) are actually failing on which Python versions -- currently the codecov failures are just leading to all jobs being cancelled in CI
This will make it easier to see in #384 which tests (if any) are actually failing on which Python versions -- currently the codecov failures are just leading to all jobs being cancelled in CI
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 :) |
Great, thanks! |
Following up from python/typing_extensions#230 (comment)! Not sure why the test for generic typeddicts on 3.11 is failing.
This: