From cfe6bbb064bc46e7e422e15ae22f467efdebe28a Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Sat, 12 Mar 2022 13:22:13 -0300 Subject: [PATCH 01/11] Avoid imports inside functions --- itemadapter/utils.py | 57 ++++++++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/itemadapter/utils.py b/itemadapter/utils.py index 787ba69..22e29f7 100644 --- a/itemadapter/utils.py +++ b/itemadapter/utils.py @@ -6,51 +6,62 @@ __all__ = ["is_item", "get_field_meta_from_class"] +try: + import scrapy +except ImportError: + scrapy = None # type: ignore [assignment] + +try: + import dataclasses +except ImportError: + dataclasses = None # type: ignore [assignment] + +try: + import attr +except ImportError: + attr = None # type: ignore [assignment] + +try: + import pydantic +except ImportError: + pydantic = None # type: ignore [assignment] + def _get_scrapy_item_classes() -> tuple: - try: - import scrapy - except ImportError: + if scrapy is None: return () - else: - try: - # handle deprecated base classes - _base_item_cls = getattr(scrapy.item, "_BaseItem", scrapy.item.BaseItem) - return (scrapy.item.Item, _base_item_cls) - except AttributeError: - return (scrapy.item.Item,) + try: + # handle deprecated base classes + _base_item_cls = getattr(scrapy.item, "_BaseItem", scrapy.item.BaseItem) + return (scrapy.item.Item, _base_item_cls) + except AttributeError: + return (scrapy.item.Item,) def _is_dataclass(obj: Any) -> bool: """In py36, this returns False if the "dataclasses" backport module is not installed.""" - try: - import dataclasses - except ImportError: + if dataclasses is None: return False return dataclasses.is_dataclass(obj) def _is_attrs_class(obj: Any) -> bool: - try: - import attr - except ImportError: + if attr is None: return False return attr.has(obj) def _is_pydantic_model(obj: Any) -> bool: - try: - from pydantic import BaseModel - except ImportError: + if pydantic is None: return False - return issubclass(obj, BaseModel) + return issubclass(obj, pydantic.BaseModel) def _get_pydantic_model_metadata(item_model: Any, field_name: str) -> MappingProxyType: metadata = {} field = item_model.__fields__[field_name].field_info - for attr in [ + for attribute in [ "alias", "title", "description", @@ -66,9 +77,9 @@ def _get_pydantic_model_metadata(item_model: Any, field_name: str) -> MappingPro "max_length", "regex", ]: - value = getattr(field, attr) + value = getattr(field, attribute) if value is not None: - metadata[attr] = value + metadata[attribute] = value if not field.allow_mutation: metadata["allow_mutation"] = field.allow_mutation metadata.update(field.extra) From 6b1a481121964bb5e5a8cca6c2657eadb6939a75 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Sat, 12 Mar 2022 13:33:55 -0300 Subject: [PATCH 02/11] Fix tests --- tests/__init__.py | 8 -------- tests/test_adapter_attrs.py | 3 +-- tests/test_adapter_dataclasses.py | 3 +-- tests/test_adapter_pydantic.py | 3 +-- tests/test_adapter_scrapy.py | 3 +-- 5 files changed, 4 insertions(+), 16 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index ace9c47..2a5d96f 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,16 +1,8 @@ -import importlib from typing import Optional from itemadapter import ItemAdapter -def mocked_import(name, *args, **kwargs): - """Allow only internal itemadapter imports.""" - if name.split(".")[0] == "itemadapter": - return importlib.__import__(name, *args, **kwargs) - raise ImportError(name) - - try: import attr except ImportError: diff --git a/tests/test_adapter_attrs.py b/tests/test_adapter_attrs.py index 435d747..eac4ce1 100644 --- a/tests/test_adapter_attrs.py +++ b/tests/test_adapter_attrs.py @@ -12,7 +12,6 @@ PydanticModel, ScrapyItem, ScrapySubclassedItem, - mocked_import, ) @@ -35,7 +34,7 @@ def test_false(self): self.assertFalse(AttrsAdapter.is_item(AttrsItem)) @unittest.skipIf(not AttrsItem, "attrs module is not available") - @mock.patch("builtins.__import__", mocked_import) + @mock.patch("itemadapter.utils.attr", None) def test_module_not_available(self): self.assertFalse(AttrsAdapter.is_item(AttrsItem(name="asdf", value=1234))) with self.assertRaises(TypeError, msg="AttrsItem is not a valid item class"): diff --git a/tests/test_adapter_dataclasses.py b/tests/test_adapter_dataclasses.py index 1b2a5c2..95c618d 100644 --- a/tests/test_adapter_dataclasses.py +++ b/tests/test_adapter_dataclasses.py @@ -12,7 +12,6 @@ PydanticModel, ScrapyItem, ScrapySubclassedItem, - mocked_import, ) @@ -35,7 +34,7 @@ def test_false(self): self.assertFalse(DataclassAdapter.is_item(DataClassItem)) @unittest.skipIf(not DataClassItem, "dataclasses module is not available") - @mock.patch("builtins.__import__", mocked_import) + @mock.patch("itemadapter.utils.dataclasses", None) def test_module_not_available(self): self.assertFalse(DataclassAdapter.is_item(DataClassItem(name="asdf", value=1234))) with self.assertRaises(TypeError, msg="DataClassItem is not a valid item class"): diff --git a/tests/test_adapter_pydantic.py b/tests/test_adapter_pydantic.py index fe14c87..20cbf40 100644 --- a/tests/test_adapter_pydantic.py +++ b/tests/test_adapter_pydantic.py @@ -13,7 +13,6 @@ PydanticSpecialCasesModel, ScrapyItem, ScrapySubclassedItem, - mocked_import, ) @@ -36,7 +35,7 @@ def test_false(self): self.assertFalse(PydanticAdapter.is_item(PydanticModel)) @unittest.skipIf(not PydanticModel, "pydantic module is not available") - @mock.patch("builtins.__import__", mocked_import) + @mock.patch("itemadapter.utils.pydantic", None) def test_module_not_available(self): self.assertFalse(PydanticAdapter.is_item(PydanticModel(name="asdf", value=1234))) with self.assertRaises(TypeError, msg="PydanticModel is not a valid item class"): diff --git a/tests/test_adapter_scrapy.py b/tests/test_adapter_scrapy.py index 4acc376..81553de 100644 --- a/tests/test_adapter_scrapy.py +++ b/tests/test_adapter_scrapy.py @@ -12,7 +12,6 @@ PydanticModel, ScrapyItem, ScrapySubclassedItem, - mocked_import, ) @@ -34,7 +33,7 @@ def test_false(self): self.assertFalse(ScrapyItemAdapter.is_item(ScrapySubclassedItem)) @unittest.skipIf(not ScrapySubclassedItem, "scrapy module is not available") - @mock.patch("builtins.__import__", mocked_import) + @mock.patch("itemadapter.utils.scrapy", None) def test_module_not_available(self): self.assertFalse(ScrapyItemAdapter.is_item(ScrapySubclassedItem(name="asdf", value=1234))) with self.assertRaises(TypeError, msg="ScrapySubclassedItem is not a valid item class"): From e898481c3ad256dd1ee3c9539ff22fe149eff1d9 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Sat, 12 Mar 2022 13:51:35 -0300 Subject: [PATCH 03/11] Test on scrapy 2.2 --- tox.ini | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index ff028c5..b0858c3 100644 --- a/tox.ini +++ b/tox.ini @@ -1,9 +1,10 @@ [tox] -envlist = bandit,flake8,typing,black,py,pylint +envlist = bandit,flake8,typing,black,py,py38-scrapy22,pylint [testenv] deps = -rtests/requirements.txt + py38-scrapy22: scrapy==2.2 commands = pytest --verbose --cov=itemadapter --cov-report=term-missing --cov-report=html --cov-report=xml --doctest-glob=README.md {posargs: itemadapter README.md tests} From a928f16a03047ff8479d4d3981b6f3765fbd1a6e Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Sat, 12 Mar 2022 16:44:18 -0300 Subject: [PATCH 04/11] Remove unnecessary else --- itemadapter/utils.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/itemadapter/utils.py b/itemadapter/utils.py index dfc4a44..9badb0c 100644 --- a/itemadapter/utils.py +++ b/itemadapter/utils.py @@ -30,14 +30,13 @@ def _get_scrapy_item_classes() -> tuple: if scrapy is None: return () + try: + # handle deprecated base classes + _base_item_cls = getattr(scrapy.item, "_BaseItem", scrapy.item.BaseItem) + except AttributeError: + return (scrapy.item.Item,) else: - try: - # handle deprecated base classes - _base_item_cls = getattr(scrapy.item, "_BaseItem", scrapy.item.BaseItem) - except AttributeError: - return (scrapy.item.Item,) - else: - return (scrapy.item.Item, _base_item_cls) + return (scrapy.item.Item, _base_item_cls) def _is_dataclass(obj: Any) -> bool: From fdee77177dc885d8191e79589c4d5c5dd983d399 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Sat, 12 Mar 2022 16:50:34 -0300 Subject: [PATCH 05/11] Fix tox command in CI --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 23d8e83..e5cc37b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -59,4 +59,4 @@ jobs: - name: Run tests env: ${{ matrix.env }} - run: tox -e py + run: tox From d6c75803d0b37188ede522663f20e40ca5da0a01 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Sat, 12 Mar 2022 17:34:51 -0300 Subject: [PATCH 06/11] Restore test coverage --- tests/__init__.py | 27 ++++++++++++++++++++++++++- tests/test_adapter_attrs.py | 19 ++++++++++++++++++- tests/test_adapter_dataclasses.py | 19 ++++++++++++++++++- tests/test_adapter_pydantic.py | 19 ++++++++++++++++++- tests/test_adapter_scrapy.py | 28 +++++++++++++++++++++++++++- 5 files changed, 107 insertions(+), 5 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index 2a5d96f..6addfa9 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,8 +1,33 @@ -from typing import Optional +import importlib +import sys +from contextlib import contextmanager +from typing import Callable, Optional from itemadapter import ItemAdapter +def make_mock_import(block_name: str) -> Callable: + def mock_import(name: str, *args, **kwargs): + """Prevent importing a specific module, let everything else pass.""" + if name.split(".")[0] == block_name: + raise ImportError(name) + return importlib.__import__(name, *args, **kwargs) + + return mock_import + + +@contextmanager +def clear_itemadapter_imports() -> None: + backup = {} + for key in sys.modules.copy().keys(): + if key.startswith("itemadapter"): + backup[key] = sys.modules.pop(key) + try: + yield + finally: + sys.modules.update(backup) + + try: import attr except ImportError: diff --git a/tests/test_adapter_attrs.py b/tests/test_adapter_attrs.py index eac4ce1..dd33222 100644 --- a/tests/test_adapter_attrs.py +++ b/tests/test_adapter_attrs.py @@ -3,7 +3,6 @@ from types import MappingProxyType from unittest import mock -from itemadapter.adapter import AttrsAdapter from itemadapter.utils import get_field_meta_from_class from tests import ( @@ -12,11 +11,15 @@ PydanticModel, ScrapyItem, ScrapySubclassedItem, + make_mock_import, + clear_itemadapter_imports, ) class AttrsTestCase(unittest.TestCase): def test_false(self): + from itemadapter.adapter import AttrsAdapter + self.assertFalse(AttrsAdapter.is_item(int)) self.assertFalse(AttrsAdapter.is_item(sum)) self.assertFalse(AttrsAdapter.is_item(1234)) @@ -33,15 +36,29 @@ def test_false(self): self.assertFalse(AttrsAdapter.is_item({"a", "set"})) self.assertFalse(AttrsAdapter.is_item(AttrsItem)) + @unittest.skipIf(not AttrsItem, "attrs module is not available") + @mock.patch("builtins.__import__", make_mock_import("attr")) + def test_module_import_error(self): + with clear_itemadapter_imports(): + from itemadapter.adapter import AttrsAdapter + + self.assertFalse(AttrsAdapter.is_item(AttrsItem(name="asdf", value=1234))) + with self.assertRaises(TypeError, msg="AttrsItem is not a valid item class"): + get_field_meta_from_class(AttrsItem, "name") + @unittest.skipIf(not AttrsItem, "attrs module is not available") @mock.patch("itemadapter.utils.attr", None) def test_module_not_available(self): + from itemadapter.adapter import AttrsAdapter + self.assertFalse(AttrsAdapter.is_item(AttrsItem(name="asdf", value=1234))) with self.assertRaises(TypeError, msg="AttrsItem is not a valid item class"): get_field_meta_from_class(AttrsItem, "name") @unittest.skipIf(not AttrsItem, "attrs module is not available") def test_true(self): + from itemadapter.adapter import AttrsAdapter + self.assertTrue(AttrsAdapter.is_item(AttrsItem())) self.assertTrue(AttrsAdapter.is_item(AttrsItem(name="asdf", value=1234))) # field metadata diff --git a/tests/test_adapter_dataclasses.py b/tests/test_adapter_dataclasses.py index 95c618d..7862005 100644 --- a/tests/test_adapter_dataclasses.py +++ b/tests/test_adapter_dataclasses.py @@ -3,7 +3,6 @@ from types import MappingProxyType from unittest import mock -from itemadapter.adapter import DataclassAdapter from itemadapter.utils import get_field_meta_from_class from tests import ( @@ -12,11 +11,15 @@ PydanticModel, ScrapyItem, ScrapySubclassedItem, + make_mock_import, + clear_itemadapter_imports, ) class DataclassTestCase(unittest.TestCase): def test_false(self): + from itemadapter.adapter import DataclassAdapter + self.assertFalse(DataclassAdapter.is_item(int)) self.assertFalse(DataclassAdapter.is_item(sum)) self.assertFalse(DataclassAdapter.is_item(1234)) @@ -33,15 +36,29 @@ def test_false(self): self.assertFalse(DataclassAdapter.is_item({"a", "set"})) self.assertFalse(DataclassAdapter.is_item(DataClassItem)) + @unittest.skipIf(not DataClassItem, "dataclasses module is not available") + @mock.patch("builtins.__import__", make_mock_import("dataclasses")) + def test_module_import_error(self): + with clear_itemadapter_imports(): + from itemadapter.adapter import DataclassAdapter + + self.assertFalse(DataclassAdapter.is_item(DataClassItem(name="asdf", value=1234))) + with self.assertRaises(TypeError, msg="DataClassItem is not a valid item class"): + get_field_meta_from_class(DataClassItem, "name") + @unittest.skipIf(not DataClassItem, "dataclasses module is not available") @mock.patch("itemadapter.utils.dataclasses", None) def test_module_not_available(self): + from itemadapter.adapter import DataclassAdapter + self.assertFalse(DataclassAdapter.is_item(DataClassItem(name="asdf", value=1234))) with self.assertRaises(TypeError, msg="DataClassItem is not a valid item class"): get_field_meta_from_class(DataClassItem, "name") @unittest.skipIf(not DataClassItem, "dataclasses module is not available") def test_true(self): + from itemadapter.adapter import DataclassAdapter + self.assertTrue(DataclassAdapter.is_item(DataClassItem())) self.assertTrue(DataclassAdapter.is_item(DataClassItem(name="asdf", value=1234))) # field metadata diff --git a/tests/test_adapter_pydantic.py b/tests/test_adapter_pydantic.py index 20cbf40..285f7cd 100644 --- a/tests/test_adapter_pydantic.py +++ b/tests/test_adapter_pydantic.py @@ -3,7 +3,6 @@ from types import MappingProxyType from unittest import mock -from itemadapter.adapter import PydanticAdapter from itemadapter.utils import get_field_meta_from_class from tests import ( @@ -13,11 +12,15 @@ PydanticSpecialCasesModel, ScrapyItem, ScrapySubclassedItem, + make_mock_import, + clear_itemadapter_imports, ) class DataclassTestCase(unittest.TestCase): def test_false(self): + from itemadapter.adapter import PydanticAdapter + self.assertFalse(PydanticAdapter.is_item(int)) self.assertFalse(PydanticAdapter.is_item(sum)) self.assertFalse(PydanticAdapter.is_item(1234)) @@ -34,15 +37,29 @@ def test_false(self): self.assertFalse(PydanticAdapter.is_item({"a", "set"})) self.assertFalse(PydanticAdapter.is_item(PydanticModel)) + @unittest.skipIf(not PydanticModel, "pydantic module is not available") + @mock.patch("builtins.__import__", make_mock_import("pydantic")) + def test_module_import_error(self): + with clear_itemadapter_imports(): + from itemadapter.adapter import PydanticAdapter + + self.assertFalse(PydanticAdapter.is_item(PydanticModel(name="asdf", value=1234))) + with self.assertRaises(TypeError, msg="PydanticModel is not a valid item class"): + get_field_meta_from_class(PydanticModel, "name") + @unittest.skipIf(not PydanticModel, "pydantic module is not available") @mock.patch("itemadapter.utils.pydantic", None) def test_module_not_available(self): + from itemadapter.adapter import PydanticAdapter + self.assertFalse(PydanticAdapter.is_item(PydanticModel(name="asdf", value=1234))) with self.assertRaises(TypeError, msg="PydanticModel is not a valid item class"): get_field_meta_from_class(PydanticModel, "name") @unittest.skipIf(not PydanticModel, "pydantic module is not available") def test_true(self): + from itemadapter.adapter import PydanticAdapter + self.assertTrue(PydanticAdapter.is_item(PydanticModel())) self.assertTrue(PydanticAdapter.is_item(PydanticModel(name="asdf", value=1234))) # field metadata diff --git a/tests/test_adapter_scrapy.py b/tests/test_adapter_scrapy.py index 81553de..658a698 100644 --- a/tests/test_adapter_scrapy.py +++ b/tests/test_adapter_scrapy.py @@ -3,7 +3,6 @@ from types import MappingProxyType from unittest import mock -from itemadapter.adapter import ScrapyItemAdapter from itemadapter.utils import get_field_meta_from_class from tests import ( @@ -12,11 +11,15 @@ PydanticModel, ScrapyItem, ScrapySubclassedItem, + make_mock_import, + clear_itemadapter_imports, ) class ScrapyItemTestCase(unittest.TestCase): def test_false(self): + from itemadapter.adapter import ScrapyItemAdapter + self.assertFalse(ScrapyItemAdapter.is_item(int)) self.assertFalse(ScrapyItemAdapter.is_item(sum)) self.assertFalse(ScrapyItemAdapter.is_item(1234)) @@ -32,15 +35,33 @@ def test_false(self): self.assertFalse(ScrapyItemAdapter.is_item({"a", "set"})) self.assertFalse(ScrapyItemAdapter.is_item(ScrapySubclassedItem)) + @unittest.skipIf(not ScrapySubclassedItem, "scrapy module is not available") + @mock.patch("builtins.__import__", make_mock_import("scrapy")) + def test_module_import_error(self): + with clear_itemadapter_imports(): + from itemadapter.adapter import ScrapyItemAdapter + + self.assertFalse( + ScrapyItemAdapter.is_item(ScrapySubclassedItem(name="asdf", value=1234)) + ) + with self.assertRaises( + TypeError, msg="ScrapySubclassedItem is not a valid item class" + ): + get_field_meta_from_class(ScrapySubclassedItem, "name") + @unittest.skipIf(not ScrapySubclassedItem, "scrapy module is not available") @mock.patch("itemadapter.utils.scrapy", None) def test_module_not_available(self): + from itemadapter.adapter import ScrapyItemAdapter + self.assertFalse(ScrapyItemAdapter.is_item(ScrapySubclassedItem(name="asdf", value=1234))) with self.assertRaises(TypeError, msg="ScrapySubclassedItem is not a valid item class"): get_field_meta_from_class(ScrapySubclassedItem, "name") @unittest.skipIf(not ScrapySubclassedItem, "scrapy module is not available") def test_true(self): + from itemadapter.adapter import ScrapyItemAdapter + self.assertTrue(ScrapyItemAdapter.is_item(ScrapyItem())) self.assertTrue(ScrapyItemAdapter.is_item(ScrapySubclassedItem())) self.assertTrue(ScrapyItemAdapter.is_item(ScrapySubclassedItem(name="asdf", value=1234))) @@ -82,6 +103,8 @@ class ScrapyDeprecatedBaseItemTestCase(unittest.TestCase): "scrapy.item._BaseItem not available", ) def test_deprecated_underscore_baseitem(self): + from itemadapter.adapter import ScrapyItemAdapter + class SubClassed_BaseItem(scrapy.item._BaseItem): pass @@ -93,6 +116,8 @@ class SubClassed_BaseItem(scrapy.item._BaseItem): "scrapy.item.BaseItem not available", ) def test_deprecated_baseitem(self): + from itemadapter.adapter import ScrapyItemAdapter + class SubClassedBaseItem(scrapy.item.BaseItem): pass @@ -102,6 +127,7 @@ class SubClassedBaseItem(scrapy.item.BaseItem): @unittest.skipIf(scrapy is None, "scrapy module is not available") def test_removed_baseitem(self): """Mock the scrapy.item module so it does not contain the deprecated _BaseItem class.""" + from itemadapter.adapter import ScrapyItemAdapter class MockItemModule: Item = ScrapyItem From 4e378c294e82d1ca8c375e0a1980e67dd79bac8c Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Sat, 12 Mar 2022 17:40:27 -0300 Subject: [PATCH 07/11] Upload coverage report for pinned env --- .github/workflows/tests.yml | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e5cc37b..602667b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -25,7 +25,10 @@ jobs: run: tox -e py - name: Upload coverage report - run: bash <(curl -s https://codecov.io/bash) + run: | + curl -Os https://uploader.codecov.io/latest/linux/codecov + chmod +x codecov + ./codecov tests-other: name: "Test: py3, ${{ matrix.os }}" @@ -33,23 +36,20 @@ jobs: strategy: matrix: include: - - python-version: 3 - os: ubuntu-latest + - os: ubuntu-latest env: TOXENV: py38-scrapy22 - - python-version: 3 - os: macos-latest + - os: macos-latest env: TOXENV: py - - python-version: 3 - os: windows-latest + - os: windows-latest env: TOXENV: py steps: - uses: actions/checkout@v2 - - name: Set up Python 3.8 + - name: Set up Python uses: actions/setup-python@v2 with: python-version: 3.8 @@ -60,3 +60,9 @@ jobs: - name: Run tests env: ${{ matrix.env }} run: tox + + - name: Upload coverage report + run: | + curl -Os https://uploader.codecov.io/latest/linux/codecov + chmod +x codecov + ./codecov From 6aaa64f3a66a57ba21498d3e8ec7196939bc6351 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Sat, 12 Mar 2022 17:46:26 -0300 Subject: [PATCH 08/11] Fix CI jobs --- .github/workflows/tests.yml | 40 ++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 602667b..45bbc0e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -31,20 +31,8 @@ jobs: ./codecov tests-other: - name: "Test: py3, ${{ matrix.os }}" - runs-on: "${{ matrix.os }}" - strategy: - matrix: - include: - - os: ubuntu-latest - env: - TOXENV: py38-scrapy22 - - os: macos-latest - env: - TOXENV: py - - os: windows-latest - env: - TOXENV: py + name: "Test: py38-scrapy22, Ubuntu" + runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 @@ -58,11 +46,31 @@ jobs: run: pip install tox - name: Run tests - env: ${{ matrix.env }} - run: tox + run: tox -e py38-scrapy22 - name: Upload coverage report run: | curl -Os https://uploader.codecov.io/latest/linux/codecov chmod +x codecov ./codecov + + tests-other-os: + name: "Test: py38, ${{ matrix.os }}" + runs-on: "${{ matrix.os }}" + strategy: + matrix: + os: [macos-latest, windows-latest] + + steps: + - uses: actions/checkout@v2 + + - name: Set up Python + uses: actions/setup-python@v2 + with: + python-version: 3.8 + + - name: Install tox + run: pip install tox + + - name: Run tests + run: tox -e py From f9852195203dbea2262a2486ecf5724520b05ee9 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Sun, 13 Mar 2022 14:46:43 -0300 Subject: [PATCH 09/11] Move optional imports to a private submodule --- itemadapter/_imports.py | 22 ++++++++++++++++++++++ itemadapter/adapter.py | 22 ++++++++++++---------- itemadapter/utils.py | 21 +-------------------- tests/test_adapter_attrs.py | 4 ++++ tests/test_adapter_dataclasses.py | 4 ++++ 5 files changed, 43 insertions(+), 30 deletions(-) create mode 100644 itemadapter/_imports.py diff --git a/itemadapter/_imports.py b/itemadapter/_imports.py new file mode 100644 index 0000000..c8667ca --- /dev/null +++ b/itemadapter/_imports.py @@ -0,0 +1,22 @@ +# attempt the following imports only once, +# to be imported from itemadapter's submodules + +try: + import scrapy # pylint: disable=W0611 (unused-import) +except ImportError: + scrapy = None # type: ignore [assignment] + +try: + import dataclasses # pylint: disable=W0611 (unused-import) +except ImportError: + dataclasses = None # type: ignore [assignment] + +try: + import attr # pylint: disable=W0611 (unused-import) +except ImportError: + attr = None # type: ignore [assignment] + +try: + import pydantic # pylint: disable=W0611 (unused-import) +except ImportError: + pydantic = None # type: ignore [assignment] diff --git a/itemadapter/adapter.py b/itemadapter/adapter.py index 273e19d..9222b16 100644 --- a/itemadapter/adapter.py +++ b/itemadapter/adapter.py @@ -12,6 +12,8 @@ _is_pydantic_model, ) +from itemadapter._imports import attr, dataclasses + __all__ = [ "AdapterInterface", @@ -100,8 +102,8 @@ def __len__(self) -> int: class AttrsAdapter(_MixinAttrsDataclassAdapter, AdapterInterface): def __init__(self, item: Any) -> None: super().__init__(item) - import attr - + if attr is None: + raise RuntimeError("attr module is not available") # store a reference to the item's fields to avoid O(n) lookups and O(n^2) traversals self._fields_dict = attr.fields_dict(self.item.__class__) @@ -115,10 +117,10 @@ def is_item_class(cls, item_class: type) -> bool: @classmethod def get_field_meta_from_class(cls, item_class: type, field_name: str) -> MappingProxyType: - from attr import fields_dict - + if attr is None: + raise RuntimeError("attr module is not available") try: - return fields_dict(item_class)[field_name].metadata # type: ignore + return attr.fields_dict(item_class)[field_name].metadata # type: ignore except KeyError: raise KeyError(f"{item_class.__name__} does not support field: {field_name}") @@ -126,8 +128,8 @@ def get_field_meta_from_class(cls, item_class: type, field_name: str) -> Mapping class DataclassAdapter(_MixinAttrsDataclassAdapter, AdapterInterface): def __init__(self, item: Any) -> None: super().__init__(item) - import dataclasses - + if dataclasses is None: + raise RuntimeError("dataclasses module is not available") # store a reference to the item's fields to avoid O(n) lookups and O(n^2) traversals self._fields_dict = {field.name: field for field in dataclasses.fields(self.item)} @@ -141,9 +143,9 @@ def is_item_class(cls, item_class: type) -> bool: @classmethod def get_field_meta_from_class(cls, item_class: type, field_name: str) -> MappingProxyType: - from dataclasses import fields - - for field in fields(item_class): + if dataclasses is None: + raise RuntimeError("dataclasses module is not available") + for field in dataclasses.fields(item_class): if field.name == field_name: return field.metadata # type: ignore raise KeyError(f"{item_class.__name__} does not support field: {field_name}") diff --git a/itemadapter/utils.py b/itemadapter/utils.py index 9badb0c..054ea34 100644 --- a/itemadapter/utils.py +++ b/itemadapter/utils.py @@ -3,29 +3,10 @@ from types import MappingProxyType from typing import Any +from itemadapter._imports import attr, dataclasses, pydantic, scrapy __all__ = ["is_item", "get_field_meta_from_class"] -try: - import scrapy -except ImportError: - scrapy = None # type: ignore [assignment] - -try: - import dataclasses -except ImportError: - dataclasses = None # type: ignore [assignment] - -try: - import attr -except ImportError: - attr = None # type: ignore [assignment] - -try: - import pydantic -except ImportError: - pydantic = None # type: ignore [assignment] - def _get_scrapy_item_classes() -> tuple: if scrapy is None: diff --git a/tests/test_adapter_attrs.py b/tests/test_adapter_attrs.py index dd33222..7aa07a2 100644 --- a/tests/test_adapter_attrs.py +++ b/tests/test_adapter_attrs.py @@ -43,6 +43,10 @@ def test_module_import_error(self): from itemadapter.adapter import AttrsAdapter self.assertFalse(AttrsAdapter.is_item(AttrsItem(name="asdf", value=1234))) + with self.assertRaises(RuntimeError, msg="attr module is not available"): + AttrsAdapter(AttrsItem(name="asdf", value=1234)) + with self.assertRaises(RuntimeError, msg="attr module is not available"): + AttrsAdapter.get_field_meta_from_class(AttrsItem, "name") with self.assertRaises(TypeError, msg="AttrsItem is not a valid item class"): get_field_meta_from_class(AttrsItem, "name") diff --git a/tests/test_adapter_dataclasses.py b/tests/test_adapter_dataclasses.py index 7862005..235a91a 100644 --- a/tests/test_adapter_dataclasses.py +++ b/tests/test_adapter_dataclasses.py @@ -43,6 +43,10 @@ def test_module_import_error(self): from itemadapter.adapter import DataclassAdapter self.assertFalse(DataclassAdapter.is_item(DataClassItem(name="asdf", value=1234))) + with self.assertRaises(RuntimeError, msg="attr module is not available"): + DataclassAdapter(DataClassItem(name="asdf", value=1234)) + with self.assertRaises(RuntimeError, msg="attr module is not available"): + DataclassAdapter.get_field_meta_from_class(DataClassItem, "name") with self.assertRaises(TypeError, msg="DataClassItem is not a valid item class"): get_field_meta_from_class(DataClassItem, "name") From eee23cf1c586f7ded27b49a538ad7cb63223f8a3 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 17 Mar 2022 12:38:28 -0300 Subject: [PATCH 10/11] Determine scrapy item classes only once --- itemadapter/_imports.py | 11 +++++++++++ itemadapter/adapter.py | 5 ++--- itemadapter/utils.py | 15 ++------------- tests/test_adapter_scrapy.py | 2 +- tox.ini | 5 +++-- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/itemadapter/_imports.py b/itemadapter/_imports.py index c8667ca..28ed9c2 100644 --- a/itemadapter/_imports.py +++ b/itemadapter/_imports.py @@ -1,10 +1,21 @@ # attempt the following imports only once, # to be imported from itemadapter's submodules +_scrapy_item_classes: tuple + try: import scrapy # pylint: disable=W0611 (unused-import) except ImportError: scrapy = None # type: ignore [assignment] + _scrapy_item_classes = () +else: + try: + # handle deprecated base classes + _base_item_cls = getattr(scrapy.item, "_BaseItem", scrapy.item.BaseItem) + except AttributeError: + _scrapy_item_classes = (scrapy.item.Item,) + else: + _scrapy_item_classes = (scrapy.item.Item, _base_item_cls) try: import dataclasses # pylint: disable=W0611 (unused-import) diff --git a/itemadapter/adapter.py b/itemadapter/adapter.py index 9222b16..d1b210b 100644 --- a/itemadapter/adapter.py +++ b/itemadapter/adapter.py @@ -6,13 +6,12 @@ from itemadapter.utils import ( _get_pydantic_model_metadata, - _get_scrapy_item_classes, _is_attrs_class, _is_dataclass, _is_pydantic_model, ) -from itemadapter._imports import attr, dataclasses +from itemadapter._imports import attr, dataclasses, _scrapy_item_classes __all__ = [ @@ -229,7 +228,7 @@ def field_names(self) -> KeysView: class ScrapyItemAdapter(_MixinDictScrapyItemAdapter, AdapterInterface): @classmethod def is_item_class(cls, item_class: type) -> bool: - return issubclass(item_class, _get_scrapy_item_classes()) + return issubclass(item_class, _scrapy_item_classes) @classmethod def get_field_meta_from_class(cls, item_class: type, field_name: str) -> MappingProxyType: diff --git a/itemadapter/utils.py b/itemadapter/utils.py index 054ea34..7d06f93 100644 --- a/itemadapter/utils.py +++ b/itemadapter/utils.py @@ -3,21 +3,10 @@ from types import MappingProxyType from typing import Any -from itemadapter._imports import attr, dataclasses, pydantic, scrapy - -__all__ = ["is_item", "get_field_meta_from_class"] +from itemadapter._imports import attr, dataclasses, pydantic -def _get_scrapy_item_classes() -> tuple: - if scrapy is None: - return () - try: - # handle deprecated base classes - _base_item_cls = getattr(scrapy.item, "_BaseItem", scrapy.item.BaseItem) - except AttributeError: - return (scrapy.item.Item,) - else: - return (scrapy.item.Item, _base_item_cls) +__all__ = ["is_item", "get_field_meta_from_class"] def _is_dataclass(obj: Any) -> bool: diff --git a/tests/test_adapter_scrapy.py b/tests/test_adapter_scrapy.py index 658a698..50d47bf 100644 --- a/tests/test_adapter_scrapy.py +++ b/tests/test_adapter_scrapy.py @@ -50,7 +50,7 @@ def test_module_import_error(self): get_field_meta_from_class(ScrapySubclassedItem, "name") @unittest.skipIf(not ScrapySubclassedItem, "scrapy module is not available") - @mock.patch("itemadapter.utils.scrapy", None) + @mock.patch("itemadapter.adapter._scrapy_item_classes", ()) def test_module_not_available(self): from itemadapter.adapter import ScrapyItemAdapter diff --git a/tox.ini b/tox.ini index b0858c3..15dd468 100644 --- a/tox.ini +++ b/tox.ini @@ -25,9 +25,10 @@ commands = [testenv:typing] basepython = python3 deps = - mypy==0.770 + mypy==0.941 commands = - mypy --show-error-codes --ignore-missing-imports --follow-imports=skip {posargs:itemadapter} + mypy --install-types --non-interactive \ + --show-error-codes --ignore-missing-imports --follow-imports=skip {posargs:itemadapter} [testenv:black] basepython = python3 From fdf13390f7db048658ebff97bfc17a93f8e2bc4a Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 17 Mar 2022 15:09:33 -0300 Subject: [PATCH 11/11] is_item class method for dict and scrapy item adapters --- itemadapter/adapter.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/itemadapter/adapter.py b/itemadapter/adapter.py index d1b210b..641b319 100644 --- a/itemadapter/adapter.py +++ b/itemadapter/adapter.py @@ -217,6 +217,10 @@ def __len__(self) -> int: class DictAdapter(_MixinDictScrapyItemAdapter, AdapterInterface): + @classmethod + def is_item(cls, item: Any) -> bool: + return isinstance(item, dict) + @classmethod def is_item_class(cls, item_class: type) -> bool: return issubclass(item_class, dict) @@ -226,6 +230,10 @@ def field_names(self) -> KeysView: class ScrapyItemAdapter(_MixinDictScrapyItemAdapter, AdapterInterface): + @classmethod + def is_item(cls, item: Any) -> bool: + return isinstance(item, _scrapy_item_classes) + @classmethod def is_item_class(cls, item_class: type) -> bool: return issubclass(item_class, _scrapy_item_classes)