Raise a better user error when failing to evaluate a forward reference #10030

Viicos · 2024-08-02T10:26:57Z

Related issue number

I first wanted to have the actual field name displayed (and possibly line number). This could be done in get_cls_type_hints_lenient (which calls eval_type_lenient for each annotation) where we have access to the field names, but this is only used by BaseModel, not Pydantic dataclasses, validate_call, etc.

But I think at least providing the bad annotation string repr as it is done in this PR could be sufficient: users just need to Ctrl + F through the code to find where it is coming from

Checklist

The pull request title is a good summary of the changes - it will be used in the changelog
Unit tests for the changes exist
Tests pass on CI
Documentation reflects the changes where applicable
My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

pydantic/_internal/_typing_extra.py

codspeed-hq · 2024-08-02T10:33:51Z

CodSpeed Performance Report

Merging #10030 will not alter performance

_{Comparing Viicos:eval-type-error (2aef05f) with main (ddc8f58)}

Summary

✅ 17 untouched benchmarks

sydney-runkle

@Viicos,

Nice work - looks solid overall, but I had one question about preserving the logical flow in general - are we missing a check for is_backport_fixable?

sydney-runkle · 2024-08-02T13:26:22Z

pydantic/_internal/_typing_extra.py

-            ) from e
-
-        return eval_type_backport(value, globalns, localns, try_default=False)
+        if isinstance(value, typing.ForwardRef):


I feel like we haven't exactly preserved the logical flow here - don't we need a is_backport_fixable_error(e)) check in the case where we jump straight to raise e?

The check is present on line 269, however the TypeError message seems to have changed in Python 3.11:

Python 3.10: TypeError: 'type' object is not subscriptable

Python 3.11+: TypeError: type 'CustomType' is not subscriptable

and is_backport_fixable_error matches the first error message. This makes things even harder to manage unfortunately :/

In other words, there's no way to distinguish between a TypeError when a builtin/stdlib class is used (e.g. list[int], collections.abc.Sequence[str]) and other classes that are truly non subscriptable

Ah gotcha. I'm ok with this approach then, but let's see what @alexmojaki thinks, as he's pretty well versed in the eval_type_backport space.

I've pushed some changes which I think is the best we can do

I suggest keeping is_backport_fixable_error, and also updating is_not_subscriptable_error to check for Python <= 3.8 and is_unsupported_types_for_union_error for Python <= 3.9 (double check those Python versions). The newer type 'CustomType' is not subscriptable format isn't backport fixable because the backport only applies to old versions. So you can definitely make a better guess about whether it's worth suggesting installing eval_type_backport.

pydantic/_internal/_typing_extra.py

alexmojaki · 2024-08-02T14:35:43Z

pydantic/_internal/_typing_extra.py

+            except TypeError as e:
+                raise TypeError(
+                    f'Unable to evaluate type annotation {value.__forward_arg__!r}. '
+                    "It might be that the type being used isn't subscriptable."


I don't think it makes sense to mention the type not being subscriptable. That should already be in the traceback with the full original error message.

Unfortunately only on 3.10+ as I mentioned here :/

So doing something like this on 3.8/3.9:

from __future__ import annotations class CustomType: pass class Model(BaseModel): foo: CustomType[int]

is quite hard to debug, as you only get TypeError: 'type' object is not subscriptable

But you will still have Unable to evaluate type annotation 'CustomType[int]'.

Ah yes makes sense

alexmojaki · 2024-08-02T15:41:03Z

This doesn't really fix #8291. The user still gets an error, they just know where it's coming from. They have no idea what to do about it. If we expect them to do some workaround such as #8291 (comment) then that should be suggested, after checking that the problem appears to be subscripting.

alexmojaki · 2024-08-02T16:29:32Z

pydantic/_internal/_typing_extra.py

+        sys.version_info <= (3, 8)
+        and msg.startswith('unsupported operand type(s) for |: ')
+        or sys.version_info <= (3, 9)
+        and "' object is not subscriptable" in msg


These versions look backwards, and that <= looks wrong.

➜ ~ python3.9 Python 3.9.19 (main, Aug 2 2024, 16:30:42) [Clang 15.0.0 (clang-1500.3.9.4)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> int | str Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: unsupported operand type(s) for |: 'type' and 'type' >>> list[int] list[int] >>> import sys >>> sys.version_info sys.version_info(major=3, minor=9, micro=19, releaselevel='final', serial=0) >>> tuple(sys.version_info) (3, 9, 19, 'final', 0) >>> sys.version_info <= (3, 9) False >>> (3, 8, 0) <= (3, 8) False

Indeed comparing with <= is confusing.

As a side note, I've raised microsoft/pyright#8648.

Viicos · 2024-08-04T12:10:46Z

This doesn't really fix #8291. The user still gets an error, they just know where it's coming from. They have no idea what to do about it. If we expect them to do some workaround such as #8291 (comment) then that should be suggested, after checking that the problem appears to be subscripting.

Not sure we can hint anything to the user about this. Even if the issue is about subscripting, it could be that:

it is generic only in stubs (as it was in the original issue)
it isn't generic at all and the user did a mistake

Defining a dummy __class_getitem__ can work but at this point I would expect the user to know what he is doing, especially because these kind of errors can only happen with custom types.

Viicos · 2024-08-05T08:14:00Z

pydantic/_internal/_typing_extra.py

+                        '`typing` constructs or install the `eval_type_backport` package.'
+                    ) from e
+
+                return eval_type_backport(value, globalns, localns, try_default=False)


On Python 3.8, we will still have the unclear TypeError: 'type' object is not subscriptable. We could add yet another try/expect block but this starts to really hurt readability here.

I'll try to figure out a better way

def eval_type_with_helpful_errors( value: Any, globalns: dict[str, Any] | None = None, localns: dict[str, Any] | None = None, type_params: tuple[Any] | None = None, ) -> Any: try: return _eval_type_with_backport_package(value, globalns, localns, type_params) except Exception as e: message = f'Unable to evaluate type annotation {value.__forward_arg__!r}.' if sys.version_info >= (3, 11): e.add_note(message) else: raise TypeError(message) from e def _eval_type_with_backport_package( value: Any, globalns: dict[str, Any] | None = None, localns: dict[str, Any] | None = None, type_params: tuple[Any] | None = None, ) -> Any: try: return _eval_type_with_type_params(value, globalns, localns, type_params) except TypeError as e: if not (isinstance(value, typing.ForwardRef) and is_backport_fixable_error(e)): raise try: from eval_type_backport import eval_type_backport except ImportError: raise TypeError( f'Unable to evaluate type annotation {value.__forward_arg__!r}. If you are making use ' 'of the new typing syntax (unions using `|` since Python 3.10 or builtins subscripting ' 'since Python 3.9), you should either replace the use of new syntax with the existing ' '`typing` constructs or install the `eval_type_backport` package.' ) from e return eval_type_backport(value, globalns, localns, try_default=False) def _eval_type_with_type_params( value: Any, globalns: dict[str, Any] | None = None, localns: dict[str, Any] | None = None, type_params: tuple[Any] | None = None, ) -> Any: if sys.version_info >= (3, 13): return typing._eval_type( # type: ignore value, globalns, localns, type_params=type_params ) else: return typing._eval_type( # type: ignore value, globalns, localns )

@Viicos I suggested except Exception in eval_type_with_helpful_errors so that there would be helpful notes for other errors. We're seeing a mysterious NameError: name 'Dict' is not defined and we don't know where it's coming from. The final code in the PR is still only catching TypeError so no note is added.

The thing is eval_type_backport is used by eval_type_lenient (defined above), and is the one responsible for catching (and not erroring) NameErrors

But eval_type_lenient is not the only place that calls eval_type_backport, so NameError can still bubble up.

raise type(e)(message) from e should work.

Viicos · 2024-08-05T20:19:26Z

Thanks @alexmojaki for the feedback. Implementation isn't that great but I don't expect this code to change (hopefully). Once we drop support for 3.8 and 3.9, it will be simplified a lot anyway.

sydney-runkle

Awesome work here 🚀 !

Raise a better user error when failing to evaluate a forward reference

3651505

github-actions bot added the relnotes-fix Used for bugfixes. label Aug 2, 2024

Viicos commented Aug 2, 2024

View reviewed changes

pydantic/_internal/_typing_extra.py Outdated Show resolved Hide resolved

sydney-runkle suggested changes Aug 2, 2024

View reviewed changes

pydantic-hooky bot added awaiting author revision awaiting changes from the PR author labels Aug 2, 2024

pydantic-hooky bot assigned Viicos Aug 2, 2024

sydney-runkle mentioned this pull request Aug 2, 2024

pydantic weekly project plan (Sydney Runkle) - July 29th, 2024 #9994

Closed

15 tasks

Handle errors differently

6c6b397

Viicos force-pushed the eval-type-error branch from 9e640d5 to c6f5b01 Compare August 2, 2024 14:14

sydney-runkle reviewed Aug 2, 2024

View reviewed changes

pydantic/_internal/_typing_extra.py Outdated Show resolved Hide resolved

Viicos added 2 commits August 2, 2024 16:29

fix test

5a902e1

Rephrase

556e897

alexmojaki reviewed Aug 2, 2024

View reviewed changes

Check for Python versions

f55fef2

alexmojaki reviewed Aug 2, 2024

View reviewed changes

Feedback

6ea8d5c

Viicos commented Aug 5, 2024

View reviewed changes

Viicos mentioned this pull request Aug 5, 2024

Enable more lint rules #10052

Merged

5 tasks

sydney-runkle mentioned this pull request Aug 5, 2024

pydantic weekly project plan (Sydney Runkle) - August 5th, 2024 #10055

Closed

8 tasks

Final fixes

0caf22f

Viicos force-pushed the eval-type-error branch from 21d9910 to 0caf22f Compare August 5, 2024 18:38

Viicos added 3 commits August 5, 2024 20:44

Fix lockfile hash

6210b1e

Format

e97c505

Even more fixes

2aef05f

alexmojaki approved these changes Aug 5, 2024

View reviewed changes

sydney-runkle approved these changes Aug 6, 2024

View reviewed changes

sydney-runkle merged commit 7e3c169 into pydantic:main Aug 6, 2024

Viicos deleted the eval-type-error branch August 6, 2024 06:02

sydney-runkle mentioned this pull request Aug 12, 2024

pydantic weekly project plan (Sydney Runkle) - August 12th, 2024 #10109

Closed

25 tasks

sydney-runkle mentioned this pull request Aug 26, 2024

pydantic weekly project plan (Sydney Runkle) - August 26th, 2024 #10236

Closed

12 tasks

zzstoatzz mentioned this pull request Aug 27, 2024

pydantic 2.9.0b1 breaks forward references in prefect PrefectHQ/prefect#15112

Closed

Viicos mentioned this pull request Oct 7, 2024

Unhelpful AssertionError when encountering a TypeError when evaluating a forward annotation since 2.9 #10565

Open

1 task

Uh oh!

Raise a better user error when failing to evaluate a forward reference #10030

Raise a better user error when failing to evaluate a forward reference #10030

Uh oh!

Conversation

Viicos commented Aug 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related issue number

Checklist

Uh oh!

Uh oh!

codspeed-hq bot commented Aug 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #10030 will not alter performance

Summary

Uh oh!

sydney-runkle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexmojaki commented Aug 2, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Viicos commented Aug 4, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Viicos Aug 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Viicos commented Aug 5, 2024

Uh oh!

sydney-runkle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Viicos commented Aug 2, 2024 •

edited

Loading

codspeed-hq bot commented Aug 2, 2024 •

edited

Loading

Viicos Aug 6, 2024 •

edited

Loading