From 941ed704012f157e800a7096279cf2297df0bc52 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 7 Oct 2024 13:55:58 +0500 Subject: [PATCH 1/6] Restore the annotated dynamic deps support. --- scrapy_poet/injection.py | 4 ++-- tests/test_injection.py | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/scrapy_poet/injection.py b/scrapy_poet/injection.py index 7bac5b68..3c7882c5 100644 --- a/scrapy_poet/injection.py +++ b/scrapy_poet/injection.py @@ -20,7 +20,7 @@ from weakref import WeakKeyDictionary import andi -from andi.typeutils import issubclass_safe +from andi.typeutils import issubclass_safe, strip_annotated from scrapy import Request, Spider from scrapy.crawler import Crawler from scrapy.http import Response @@ -247,7 +247,7 @@ def _get_dynamic_deps_factory( """ ns: Dict[str, type] = {} for type_ in dynamic_types: - if not isinstance(type_, type): + if not isinstance(strip_annotated(type_), type): raise TypeError(f"Expected a dynamic dependency type, got {type_!r}") ns[type_.__name__] = type_ txt = Injector._get_dynamic_deps_factory_text(ns.keys()) diff --git a/tests/test_injection.py b/tests/test_injection.py index 90f03104..2ee4cafd 100644 --- a/tests/test_injection.py +++ b/tests/test_injection.py @@ -989,6 +989,23 @@ def test_dynamic_deps_factory(): assert dd == {int: 42, Cls1: c} +@pytest.mark.skipif( + sys.version_info < (3, 9), reason="No Annotated support in Python < 3.9" +) +def test_dynamic_deps_factory_annotated(): + from typing import Annotated + + fn = Injector._get_dynamic_deps_factory([Annotated[Cls1, 42]]) + args = andi.inspect(fn) + # the arg name needs to be fixed separately + assert args == { + "Annotated_arg": [Annotated[Cls1, 42]], + } + c1 = Cls1() + dd = fn(Annotated_arg=c1) + assert dd == {Annotated[Cls1, 42]: c1} + + def test_dynamic_deps_factory_bad_input(): with pytest.raises( TypeError, From f6c644bebdc9ae6ed30d7861fcf0a9ddd85b9ff4 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 7 Oct 2024 15:29:02 +0500 Subject: [PATCH 2/6] A better fix with 3.9 support. --- scrapy_poet/injection.py | 3 ++- tests/test_injection.py | 13 ++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/scrapy_poet/injection.py b/scrapy_poet/injection.py index 3c7882c5..162ae295 100644 --- a/scrapy_poet/injection.py +++ b/scrapy_poet/injection.py @@ -247,7 +247,8 @@ def _get_dynamic_deps_factory( """ ns: Dict[str, type] = {} for type_ in dynamic_types: - if not isinstance(strip_annotated(type_), type): + type_ = strip_annotated(type_) + if not isinstance(type_, type): raise TypeError(f"Expected a dynamic dependency type, got {type_!r}") ns[type_.__name__] = type_ txt = Injector._get_dynamic_deps_factory_text(ns.keys()) diff --git a/tests/test_injection.py b/tests/test_injection.py index 2ee4cafd..dd832b76 100644 --- a/tests/test_injection.py +++ b/tests/test_injection.py @@ -995,15 +995,18 @@ def test_dynamic_deps_factory(): def test_dynamic_deps_factory_annotated(): from typing import Annotated - fn = Injector._get_dynamic_deps_factory([Annotated[Cls1, 42]]) + fn = Injector._get_dynamic_deps_factory( + [Annotated[Cls1, 42], Annotated[Cls2, "foo"]] + ) args = andi.inspect(fn) - # the arg name needs to be fixed separately assert args == { - "Annotated_arg": [Annotated[Cls1, 42]], + "Cls1_arg": [Annotated[Cls1, 42]], + "Cls2_arg": [Annotated[Cls2, "foo"]], } c1 = Cls1() - dd = fn(Annotated_arg=c1) - assert dd == {Annotated[Cls1, 42]: c1} + c2 = Cls2() + dd = fn(Cls1_arg=c1, Cls2_arg=c2) + assert dd == {Annotated[Cls1, 42]: c1, Annotated[Cls2, "foo"]: c2} def test_dynamic_deps_factory_bad_input(): From e9f8a434b7ade9ce70da057dd432bd099d63800f Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 7 Oct 2024 15:31:25 +0500 Subject: [PATCH 3/6] Add the full annotated dynamic deps test with the current behavior. --- scrapy_poet/injection.py | 2 +- tests/test_injection.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/scrapy_poet/injection.py b/scrapy_poet/injection.py index 162ae295..b558f8b5 100644 --- a/scrapy_poet/injection.py +++ b/scrapy_poet/injection.py @@ -247,7 +247,7 @@ def _get_dynamic_deps_factory( """ ns: Dict[str, type] = {} for type_ in dynamic_types: - type_ = strip_annotated(type_) + type_ = cast(type, strip_annotated(type_)) if not isinstance(type_, type): raise TypeError(f"Expected a dynamic dependency type, got {type_!r}") ns[type_.__name__] = type_ diff --git a/tests/test_injection.py b/tests/test_injection.py index dd832b76..2e92b88e 100644 --- a/tests/test_injection.py +++ b/tests/test_injection.py @@ -666,6 +666,39 @@ def callback(dd: DynamicDeps): instances = yield from injector.build_instances(request, response, plan) assert set(instances) == {TestItemPage, TestItem, DynamicDeps} + @pytest.mark.skipif( + sys.version_info < (3, 9), reason="No Annotated support in Python < 3.9" + ) + @inlineCallbacks + def test_dynamic_deps_annotated(self): + from typing import Annotated + + def callback(dd: DynamicDeps): + pass + + provider = get_provider({Cls1, Cls2}) + injector = get_injector_for_testing({provider: 1}) + + expected_instances = { + DynamicDeps: DynamicDeps( + {Annotated[Cls1, 42]: Cls1(), Annotated[Cls2, "foo"]: Cls2()} + ), + Annotated[Cls1, 42]: Cls1(), + Annotated[Cls2, "foo"]: Cls2(), + } + expected_kwargs = { + "dd": DynamicDeps( + {Annotated[Cls1, 42]: Cls1(), Annotated[Cls2, "foo"]: Cls2()} + ), + } + yield self._assert_instances( + injector, + callback, + expected_instances, + expected_kwargs, + reqmeta={"inject": [Annotated[Cls1, 42], Annotated[Cls2, "foo"]]}, + ) + class Html(Injectable): url = "http://example.com" From fb571913a12dc133da34f9aa78a8820e7362026f Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 7 Oct 2024 20:05:33 +0500 Subject: [PATCH 4/6] Small refactoring of _get_dynamic_deps_factory(). --- scrapy_poet/injection.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scrapy_poet/injection.py b/scrapy_poet/injection.py index b558f8b5..8a07a46a 100644 --- a/scrapy_poet/injection.py +++ b/scrapy_poet/injection.py @@ -245,13 +245,14 @@ def _get_dynamic_deps_factory( corresponding args. It has correct type hints so that it can be used as an ``andi`` custom builder. """ - ns: Dict[str, type] = {} + type_names: List[str] = [] for type_ in dynamic_types: type_ = cast(type, strip_annotated(type_)) if not isinstance(type_, type): raise TypeError(f"Expected a dynamic dependency type, got {type_!r}") - ns[type_.__name__] = type_ - txt = Injector._get_dynamic_deps_factory_text(ns.keys()) + type_names.append(type_.__name__) + txt = Injector._get_dynamic_deps_factory_text(type_names) + ns: Dict[str, Any] = {} exec(txt, globals(), ns) return ns["__create_fn__"](*dynamic_types) From 6d2e075ffea91df3b2baaf631b55c3b4acf4b33c Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 8 Oct 2024 13:45:19 +0500 Subject: [PATCH 5/6] Put unannotated types into DynamicDeps keys. --- scrapy_poet/injection.py | 2 +- tests/test_injection.py | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/scrapy_poet/injection.py b/scrapy_poet/injection.py index 8a07a46a..2e8189f0 100644 --- a/scrapy_poet/injection.py +++ b/scrapy_poet/injection.py @@ -224,7 +224,7 @@ def _get_dynamic_deps_factory_text( # https://github.com/python/cpython/blob/v3.11.9/Lib/dataclasses.py#L413 args = [f"{name}_arg: {name}" for name in type_names] args_str = ", ".join(args) - result_args = [f"{name}: {name}_arg" for name in type_names] + result_args = [f"strip_annotated({name}): {name}_arg" for name in type_names] result_args_str = ", ".join(result_args) create_args_str = ", ".join(type_names) return ( diff --git a/tests/test_injection.py b/tests/test_injection.py index 2e92b88e..068bcc09 100644 --- a/tests/test_injection.py +++ b/tests/test_injection.py @@ -680,16 +680,12 @@ def callback(dd: DynamicDeps): injector = get_injector_for_testing({provider: 1}) expected_instances = { - DynamicDeps: DynamicDeps( - {Annotated[Cls1, 42]: Cls1(), Annotated[Cls2, "foo"]: Cls2()} - ), + DynamicDeps: DynamicDeps({Cls1: Cls1(), Cls2: Cls2()}), Annotated[Cls1, 42]: Cls1(), Annotated[Cls2, "foo"]: Cls2(), } expected_kwargs = { - "dd": DynamicDeps( - {Annotated[Cls1, 42]: Cls1(), Annotated[Cls2, "foo"]: Cls2()} - ), + "dd": DynamicDeps({Cls1: Cls1(), Cls2: Cls2()}), } yield self._assert_instances( injector, From 0e5cc9303ef125a014171c31e62952beae9ab49d Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 8 Oct 2024 13:53:07 +0500 Subject: [PATCH 6/6] Fix tests. --- tests/test_injection.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_injection.py b/tests/test_injection.py index 068bcc09..a2a216e5 100644 --- a/tests/test_injection.py +++ b/tests/test_injection.py @@ -1001,7 +1001,7 @@ def test_dynamic_deps_factory_text(): txt == """def __create_fn__(int, Cls1): def dynamic_deps_factory(int_arg: int, Cls1_arg: Cls1) -> DynamicDeps: - return DynamicDeps({int: int_arg, Cls1: Cls1_arg}) + return DynamicDeps({strip_annotated(int): int_arg, strip_annotated(Cls1): Cls1_arg}) return dynamic_deps_factory""" ) @@ -1035,7 +1035,7 @@ def test_dynamic_deps_factory_annotated(): c1 = Cls1() c2 = Cls2() dd = fn(Cls1_arg=c1, Cls2_arg=c2) - assert dd == {Annotated[Cls1, 42]: c1, Annotated[Cls2, "foo"]: c2} + assert dd == {Cls1: c1, Cls2: c2} def test_dynamic_deps_factory_bad_input():