-
-
Notifications
You must be signed in to change notification settings - Fork 67
Fix alru_cache crash with deferred annotations on Python 3.14 #781
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
base: master
Are you sure you want to change the base?
Changes from 4 commits
d098e0f
60fa4fe
235da6c
140a4b8
73906f3
26f7eda
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -99,10 +99,17 @@ def __init__( | |
| self.__doc__ = fn.__doc__ | ||
| except AttributeError: | ||
| pass | ||
| # Python 3.14+ (PEP 649): copy the lazy __annotate__ function the | ||
| # way functools.update_wrapper does; reading fn.__annotations__ | ||
| # would force evaluation of deferred annotations and fail on | ||
| # names that are not defined yet. | ||
| try: | ||
| self.__annotations__ = fn.__annotations__ | ||
| self.__annotate__ = fn.__annotate__ # type: ignore[attr-defined] | ||
| except AttributeError: | ||
| pass | ||
| try: | ||
| self.__annotations__ = fn.__annotations__ | ||
| except AttributeError: | ||
| pass | ||
| try: | ||
| self.__dict__.update(fn.__dict__) | ||
| except AttributeError: | ||
|
|
@@ -329,10 +336,15 @@ def __init__( | |
| self.__doc__ = wrapper.__doc__ | ||
| except AttributeError: | ||
| pass | ||
| # Python 3.14+ (PEP 649): prefer the lazy __annotate__ function, | ||
| # see the matching logic in _LRUCacheWrapper.__init__. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should really make both comments concise like this. I'd also drop the prefix so there's less maintenance. We can see the version check below.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 26f7eda — both comments trimmed, prefix dropped. |
||
| try: | ||
| self.__annotations__ = wrapper.__annotations__ | ||
| self.__annotate__ = wrapper.__annotate__ | ||
| except AttributeError: | ||
| pass | ||
| try: | ||
| self.__annotations__ = wrapper.__annotations__ | ||
| except AttributeError: | ||
| pass | ||
| try: | ||
| self.__dict__.update(wrapper.__dict__) | ||
| except AttributeError: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| """Tests for PEP 649 deferred annotations (Python 3.14+). | ||
|
|
||
| On Python 3.14 annotations are evaluated lazily via ``__annotate__``. | ||
| Copying ``fn.__annotations__`` eagerly at decoration time forces that | ||
| evaluation and crashes with ``NameError`` when the annotation refers to | ||
| a name defined after the decorated function, a pattern that works with | ||
| ``functools.lru_cache``. | ||
| """ | ||
|
|
||
| import inspect | ||
| import sys | ||
| from functools import lru_cache, partial | ||
|
|
||
| import pytest | ||
|
|
||
| from async_lru import alru_cache | ||
|
|
||
|
|
||
| requires_py314 = pytest.mark.skipif( | ||
| sys.version_info < (3, 14), | ||
| reason="deferred annotation evaluation requires Python 3.14", | ||
| ) | ||
|
|
||
|
|
||
| @requires_py314 | ||
| async def test_deco_with_annotation_defined_after() -> None: | ||
| @alru_cache(maxsize=1) | ||
| async def get_foo() -> Foo: | ||
| return Foo() | ||
|
|
||
| class Foo: | ||
| pass | ||
|
|
||
| first = await get_foo() | ||
| assert isinstance(first, Foo) | ||
| assert await get_foo() is first | ||
|
|
||
|
|
||
| @requires_py314 | ||
| async def test_annotations_stay_lazy_like_lru_cache() -> None: | ||
| @alru_cache(maxsize=1) | ||
| async def get_foo_async() -> Foo: | ||
| return Foo() | ||
|
|
||
| @lru_cache(maxsize=1) | ||
| def get_foo_sync() -> Foo: | ||
| return Foo() | ||
|
|
||
| class Foo: | ||
| pass | ||
|
|
||
| assert isinstance(await get_foo_async(), Foo) | ||
| assert isinstance(get_foo_sync(), Foo) | ||
| assert ( | ||
| inspect.get_annotations(get_foo_async) | ||
| == inspect.get_annotations(get_foo_sync) | ||
| == {"return": Foo} | ||
| ) | ||
|
|
||
|
|
||
| @requires_py314 | ||
| async def test_unresolvable_annotation_raises_only_on_access() -> None: | ||
| @alru_cache(maxsize=1) | ||
| async def broken() -> Missing: # type: ignore[name-defined] # noqa: F821 | ||
| pass # pragma: no cover | ||
|
|
||
| with pytest.raises(NameError): | ||
| inspect.get_annotations(broken) | ||
|
|
||
|
|
||
| async def test_method_wrapping_partial_without_annotation_attributes() -> None: | ||
| """A wrapped ``partial`` carries neither ``__annotate__`` nor | ||
| ``__annotations__``; binding it as a method copies neither.""" | ||
|
|
||
| async def impl(self: object) -> int: | ||
| return 42 | ||
|
|
||
| class Api: | ||
| meth = alru_cache(partial(impl)) | ||
|
|
||
| api = Api() | ||
| assert await api.meth() == 42 | ||
| assert not hasattr(api.meth, "__annotations__") | ||
|
|
||
|
|
||
| @requires_py314 | ||
| async def test_method_annotations_stay_lazy() -> None: | ||
| class Api: | ||
| @alru_cache(maxsize=1) | ||
| async def get_foo(self) -> Foo: | ||
| return Foo() | ||
|
|
||
| class Foo: | ||
| pass | ||
|
|
||
| api = Api() | ||
| assert isinstance(await api.get_foo(), Foo) | ||
| assert inspect.get_annotations(api.get_foo) == {"return": Foo} |
Uh oh!
There was an error while loading. Please reload this page.
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.
@rodrigobnogueira I would recommend using
sys.version_infoas a faster workaround for this problem as this is 3.14+ related, this way this part of the code doesn't trigger a performance regression.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.
Done in 73906f3. Both copy sites now branch on
sys.version_info >= (3, 14); older interpreters do the plain__annotations__copy with no try/except on a missing__annotate__. The narrow try/except stays inside the 3.14 branch for wrapped partials, which the partial test covers. Thetype: ignore[attr-defined]became unused under the gate and was removed. Suite green on 3.12 and 3.14, mypy clean on both.