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

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

Closed
github-actions bot opened this issue Oct 25, 2024 · 15 comments
Closed

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

github-actions bot opened this issue Oct 25, 2024 · 15 comments

Comments

@github-actions
Copy link

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

@hauntsaninja
Copy link
Collaborator

There's a chance this one is real

@AlexWaygood
Copy link
Member

A pydantic error on py312 and py313 that looks like it might have something to do with type aliases — could well be related to 139ac68

@hauntsaninja
Copy link
Collaborator

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

@Daraan
Copy link
Contributor

Daraan commented Oct 25, 2024

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
Copy link
Member

AlexWaygood commented Oct 25, 2024

(@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
Copy link
Contributor

Daraan commented Oct 25, 2024

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 pydantic/pydantic#10530 two weeks ago
The tests/test_edge_cases.py:2875 test_nested_type_statement: pydantic/pydantic#10369

There with comment:

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

@Daraan
Copy link
Contributor

Daraan commented Oct 25, 2024

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 pydantic/pydantic#10369 the two statements that fail here pass without errors on 4.12.2 but not on the latest version.

@AlexWaygood
Copy link
Member

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

@Daraan
Copy link
Contributor

Daraan commented Oct 25, 2024

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
Copy link
Contributor

Daraan commented Oct 25, 2024

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__?

  • As written in #477 creating a typing.TypeAliasType during typing_extensions.TypeAliasType.__new__ fails the pickle test. I do not know how that could be solved.

@AlexWaygood
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor

Daraan commented Oct 25, 2024

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
Copy link
Member

Fixed in Pydantic, thanks @Daraan and @Viicos!

@Viicos
Copy link
Contributor

Viicos commented Nov 12, 2024

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

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

No branches or pull requests

5 participants