python / typing_extensions

Backported and experimental type hints for Python
Other
446 stars 110 forks source link

Third-party tests failed on Fri Oct 25 2024 #493

Closed github-actions[bot] closed 1 month ago

github-actions[bot] commented 1 month ago

Runs listed here: https://github.com/python/typing_extensions/actions/workflows/third_party.yml

hauntsaninja commented 1 month ago

There's a chance this one is real

AlexWaygood commented 1 month ago

A pydantic error on py312 and py313 that looks like it might have something to do with type aliases — could well be related to https://github.com/python/typing_extensions/commit/139ac686aab0fc4ead81d3d1d199f386977a1532

hauntsaninja commented 1 month ago

cc @Daraan in case you have time to look into it :-)

Daraan commented 1 month ago

Im already looking, but I am not very familiar with pydantic and do not see how the type statement is interpreted here. The last PR back ported TypeAliasType also for 3.12 &3.13 which definitely is related.

I'll look again in the next hours

AlexWaygood commented 1 month ago

(@Daraan if it was caused by your change, this doesn't necessarily mean there was something wrong with your PR! Pydantic might be making an incorrect assumption somewhere. But we should at least try to track down the cause so that we can at least let them know before we make our next release :-)

Daraan commented 1 month ago

I now tested the first pydantic code in question in a new 3.12 & 3.13 environment and it also fails with the latest release.

Edit:

The test_forward_ref.py:1261 test_class_locals_are_kept_during_schema_generation was introduced with https://github.com/pydantic/pydantic/pull/10530 two weeks ago The tests/test_edge_cases.py:2875 test_nested_type_statement: https://github.com/pydantic/pydantic/pull/10369

There with comment:

currently will throws an error because TypeAliasType is not in default_ignored_types()

Daraan commented 1 month ago

I've now looked into it, this is an issue on the current pydantic v2.9.2 (2024-09-17) but with the changes from https://github.com/pydantic/pydantic/pull/10369 the two statements that fail here pass without errors on 4.12.2 but not on the latest version.

AlexWaygood commented 1 month ago

Hmm, I thought our third-party tests workflow ran pydantic's tests using the pydantic main branch rather than a release of pydantic?

Daraan commented 1 month ago

Today I learned --force-reinstall or uninstall is required when using pip install git+https://github.com/python/typing_extensions.git --upgrade and typing_extensions is already present.

Now the error is back with pydantic main

Daraan commented 1 month ago

Finally, The problem in pydantic can be minimally written as this:

from typing_extensions import TypeAliasType
type Int = int  # creates typing.TypeAliasType
assert isinstance(Int, TypeAliasType) # fails

I see two solutions: 1) pydantic side: check for isinstance(Int, (typing.TypeAliasType, typing_extensions.TypeAliasType)) 2) typing_extensions: add __isinstancecheck__ to pass isintance(typing_instance, typing_extensions.TypeAliasType). See possible fix here

The problem with 1) is that all 3rd party maintainers need to be aware of checking the two different versions. The problem with 2) I currently miss an idea how to pass isinstance(typing_extensions_instance, typing.TypeAliasType); and should these tests pass that way?

Possible solution with __new__?

AlexWaygood commented 1 month ago

I suspected it might be something like this. We have a warning about this kind of thing in our docs, but as you say, it's very easy for third-party libraries to forget to watch out for this footgun: https://typing-extensions.readthedocs.io/en/latest/#runtime-use-of-types

I'm not sure your solution (2) is possible on the typing_extensions side -- we'd need to add __instancecheck__ to the metaclass of typing.TypeAliasType :/

I don't think a solution doing something fancy with __new__ (like we do with typing_extensions.TypeVar and similar) works either because typing.TypeAliasType instances are immutable.

So we may need to patch this over at pydantic and loudly communicate the change in our changelog.

JelleZijlstra commented 1 month ago

Thanks for figuring this out! We already document at https://typing-extensions.readthedocs.io/en/latest/#runtime-use-of-types that you should always test both the typing and typing-extensions versions of types, even if they're currently the same. So pydantic should be changed to do so here; is anyone interested in sending an issue and/or a PR over to them to do so?

Daraan commented 1 month ago

So pydantic should be changed to do so here; is anyone interested in sending an issue and/or a PR over to them to do so?

Done, see pydantic Issue and PR draft.

JelleZijlstra commented 1 month ago

Fixed in Pydantic, thanks @Daraan and @Viicos!

Viicos commented 2 weeks ago

I recently refactored our typing inspection utilities, so we shouldn't get any new issues like these hopefully.