diff --git a/.flake8 b/.flake8 deleted file mode 100644 index 1d317e49..00000000 --- a/.flake8 +++ /dev/null @@ -1,53 +0,0 @@ -[flake8] -ignore = - # Refers to the max-line length. Let's suppress the error and simply - # let black take care on how it wants to format the lines. - E501, - - # Refers to "line break before binary operator". - # Similar to above, let black take care of the formatting. - W503, - - # Refers to "necessary dict call - rewrite as a literal". - C408, - - # To be addressed: - # Missing docstring in public module - D100, - # Missing docstring in public class - D101, - # Missing docstring in public method - D102, - # Missing docstring in public function - D103, - # Missing docstring in public package - D104, - # Missing docstring in magic method - D105, - # Missing docstring in __init__ - D107, - # One-line docstring should fit on one line with quotes - D200, - # No blank lines allowed after function docstring - D202, - # 1 blank line required between summary line and description - D205, - # Multi-line docstring closing quotes should be on a separate line - D209, - # First line should end with a period - D400, - # First line should be in imperative mood - D401, - # First line should not be the function's "signature" - D402 - -per-file-ignores = - # F401: Ignore "imported but unused" errors in __init__ files, as those - # imports are there to expose submodule functions so they can be imported - # directly from that module - # F403: Ignore * imports in these files - scrapy_poet/__init__.py:F401,F403 - scrapy_poet/page_inputs/__init__.py:F401,F403 - - # false positive in one case; other cases are fixed - scrapy_poet/injection.py:B028 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5aa77fbf..616676b8 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,22 +1,7 @@ repos: - - hooks: - - id: black - language_version: python3 - repo: https://github.com/ambv/black - rev: 24.10.0 - - hooks: - - id: isort - language_version: python3 - repo: https://github.com/PyCQA/isort - rev: 5.13.2 - - hooks: - - id: flake8 - language_version: python3 - additional_dependencies: - - flake8-bugbear - - flake8-comprehensions - - flake8-debugger - - flake8-docstrings - - flake8-string-format - repo: https://github.com/pycqa/flake8 - rev: 7.1.1 +- repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.13.1 + hooks: + - id: ruff-check + args: [ --fix ] + - id: ruff-format diff --git a/docs/conf.py b/docs/conf.py index 1f3571b0..a834283f 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -10,13 +10,12 @@ # add these directories to sys.path here. If the directory is relative to the # documentation root, use os.path.abspath to make it absolute, like shown here. # -import os import pkgutil import sys from datetime import datetime from pathlib import Path -sys.path.insert(0, os.path.abspath("../")) +sys.path.insert(0, str(Path(__file__).parent.parent)) def get_copyright(attribution, *, first_year): @@ -29,7 +28,7 @@ def get_copyright(attribution, *, first_year): def get_version_and_release(): try: - import scrapy_poet # noqa: F401 + import scrapy_poet # noqa: F401,PLC0415 except ImportError: return "", "" version_bytes = pkgutil.get_data("scrapy_poet", "VERSION") or b"" @@ -42,7 +41,7 @@ def get_version_and_release(): # -- Project information ----------------------------------------------------- project = "scrapy-poet" -copyright = get_copyright("Zyte Group Ltd", first_year=2019) +project_copyright = get_copyright("Zyte Group Ltd", first_year=2019) author = "Zyte" version, release = get_version_and_release() diff --git a/example/example/autoextract.py b/example/example/autoextract.py index 816056cd..7ba8917e 100644 --- a/example/example/autoextract.py +++ b/example/example/autoextract.py @@ -3,7 +3,7 @@ which even requires an API request. """ -from typing import Any, Dict +from typing import Any import attr from scrapy import Request @@ -18,7 +18,7 @@ class AutoextractProductResponse: """Input data""" - data: Dict[str, Any] + data: dict[str, Any] class AutoextractProductProvider(PageObjectInputProvider): @@ -51,5 +51,4 @@ def url(self): return self.autoextract_resp.data["product"]["url"] def to_item(self): - product = self.autoextract_resp.data["product"] - return product + return self.autoextract_resp.data["product"] diff --git a/example/example/spiders/books_03.py b/example/example/spiders/books_03.py index 274eb211..a7d388a6 100644 --- a/example/example/spiders/books_03.py +++ b/example/example/spiders/books_03.py @@ -3,8 +3,8 @@ """ import scrapy -from example.autoextract import ProductPage +from example.autoextract import ProductPage from scrapy_poet import callback_for diff --git a/example/example/spiders/books_05.py b/example/example/spiders/books_05.py index 347a98a3..f474eb13 100644 --- a/example/example/spiders/books_05.py +++ b/example/example/spiders/books_05.py @@ -4,9 +4,10 @@ """ import scrapy -from example.autoextract import ProductPage from web_poet import WebPage +from example.autoextract import ProductPage + class BookListPage(WebPage): def product_urls(self): diff --git a/example/example/spiders/books_05_1.py b/example/example/spiders/books_05_1.py index 575cd057..39e8a303 100644 --- a/example/example/spiders/books_05_1.py +++ b/example/example/spiders/books_05_1.py @@ -12,9 +12,9 @@ """ import scrapy -from example.autoextract import ProductPage from web_poet import WebPage +from example.autoextract import ProductPage from scrapy_poet import DummyResponse diff --git a/pyproject.toml b/pyproject.toml index 8d360e16..a95d21b2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,3 @@ -[tool.black] -line-length = 88 - [tool.bumpversion] current_version = "0.26.0" commit = true @@ -34,12 +31,6 @@ exclude_also = [ "@(abc\\.)?abstractmethod", ] -[tool.isort] -profile = "black" -multi_line_output = 3 -# scrapy_poet/__init__.py: Automatic sorting causes circular dependencies. -skip = ["scrapy_poet/__init__.py"] - [[tool.mypy.overrides]] module = [ "tests.test_cache.*", @@ -51,3 +42,154 @@ module = [ # when test cases are decorated with @inlineCallbacks. However, the # tests doesn't return anything at all. disable_error_code = "misc" + +[tool.ruff.lint] +extend-select = [ + # flake8-builtins + "A", + # flake8-async + "ASYNC", + # flake8-bugbear + "B", + # flake8-comprehensions + "C4", + # flake8-commas + "COM", + # pydocstyle + "D", + # flake8-future-annotations + "FA", + # flynt + "FLY", + # refurb + "FURB", + # isort + "I", + # flake8-implicit-str-concat + "ISC", + # flake8-logging + "LOG", + # Perflint + "PERF", + # pygrep-hooks + "PGH", + # flake8-pie + "PIE", + # pylint + "PL", + # flake8-pytest-style + "PT", + # flake8-use-pathlib + "PTH", + # flake8-pyi + "PYI", + # flake8-quotes + "Q", + # flake8-return + "RET", + # flake8-raise + "RSE", + # Ruff-specific rules + "RUF", + # flake8-bandit + "S", + # flake8-simplify + "SIM", + # flake8-slots + "SLOT", + # flake8-debugger + "T10", + # flake8-type-checking + "TC", + # flake8-tidy-imports + "TID", + # pyupgrade + "UP", + # pycodestyle warnings + "W", + # flake8-2020 + "YTT", +] +ignore = [ + # Trailing comma missing + "COM812", + # Missing docstring in public module + "D100", + # Missing docstring in public class + "D101", + # Missing docstring in public method + "D102", + # Missing docstring in public function + "D103", + # Missing docstring in public package + "D104", + # Missing docstring in magic method + "D105", + # Missing docstring in __init__ + "D107", + # One-line docstring should fit on one line with quotes + "D200", + # No blank lines allowed after function docstring + "D202", + # 1 blank line required between summary line and description + "D205", + # Multi-line docstring closing quotes should be on a separate line + "D209", + # First line should end with a period + "D400", + # First line should be in imperative mood; try rephrasing + "D401", + # First line should not be the function's "signature" + "D402", + # Too many return statements + "PLR0911", + # Too many branches + "PLR0912", + # Too many arguments in function definition + "PLR0913", + # Too many statements + "PLR0915", + # Magic value used in comparison + "PLR2004", + # String contains ambiguous {}. + "RUF001", + # Docstring contains ambiguous {}. + "RUF002", + # Comment contains ambiguous {}. + "RUF003", + # Mutable class attributes should be annotated with `typing.ClassVar` + "RUF012", + # Use of `assert` detected + "S101", + # Yoda condition detected + "SIM300", + # Add `from __future__ import annotations` to simplify + # (It's harder to keep annotations resolvable at the runtime with it.) + "FA100", +] + +[tool.ruff.lint.flake8-tidy-imports] +banned-module-level-imports = [ + "twisted.internet.reactor", +] + +[tool.ruff.lint.isort] +split-on-trailing-comma = false + +[tool.ruff.lint.per-file-ignores] +"example/*" = ["PLC0415"] +# scrapy_poet/__init__.py: Automatic import sorting causes circular dependencies. +"scrapy_poet/__init__.py" = ["F401", "I"] +"scrapy_poet/page_inputs/__init__.py" = ["F401"] +"tests/*" = ["SLOT000", "S"] + +# we need to use typing.Set[] over modern alternatives with web-poet<0.19.0 && Python<3.11 +# see https://github.com/scrapinghub/web-poet/pull/219 +"scrapy_poet/page_input_providers.py" = ["UP006", "UP035"] +"tests/test_downloader.py" =["UP006", "UP035"] +"tests/test_providers.py" =["UP006", "UP035"] +"tests/test_request_fingerprinter.py" =["UP006", "UP035"] +"tests/test_web_poet_rules.py" =["UP006", "UP035"] + +[tool.ruff.lint.pydocstyle] +convention = "pep257" diff --git a/scrapy_poet/_addon.py b/scrapy_poet/_addon.py index b75de290..2f07ec0e 100644 --- a/scrapy_poet/_addon.py +++ b/scrapy_poet/_addon.py @@ -22,15 +22,15 @@ def _replace_builtin( f"{builtin_cls} entry with {new_cls}. Add {new_cls} manually to " f"silence this warning." ) - return None + return if new_cls in setting_value: - return None + return for cls_or_path in setting_value: if isinstance(cls_or_path, str): _cls = load_object(cls_or_path) if _cls == new_cls: - return None + return builtin_entry: object = None for _setting_value in (setting_value, settings[f"{setting}_BASE"]): @@ -54,7 +54,7 @@ def _replace_builtin( f"missing built-in entry {builtin_cls}. Cannot replace it with {new_cls}. " f"Add {new_cls} manually to silence this warning." ) - return None + return if pos is None: logger.warning( diff --git a/scrapy_poet/_request_fingerprinter.py b/scrapy_poet/_request_fingerprinter.py index ba24b41d..056d8a26 100644 --- a/scrapy_poet/_request_fingerprinter.py +++ b/scrapy_poet/_request_fingerprinter.py @@ -1,8 +1,10 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + try: - from scrapy.utils.request import RequestFingerprinter # NOQA + from scrapy.utils.request import RequestFingerprinter # noqa: F401 except ImportError: - from typing import TYPE_CHECKING - if not TYPE_CHECKING: ScrapyPoetRequestFingerprinter = None else: @@ -10,15 +12,17 @@ import json from functools import cached_property from logging import getLogger - from typing import Annotated, Callable, Dict, List, Optional, get_args, get_origin + from typing import Annotated, Callable, get_args, get_origin from weakref import WeakKeyDictionary from andi import CustomBuilder - from scrapy import Request - from scrapy.crawler import Crawler from scrapy.settings.default_settings import REQUEST_FINGERPRINTER_CLASS from scrapy.utils.misc import load_object + if TYPE_CHECKING: + from scrapy import Request + from scrapy.crawler import Crawler + try: from scrapy.utils.misc import build_from_crawler except ImportError: # Scrapy < 2.12 @@ -52,14 +56,12 @@ def build_from_crawler( def _serialize_dep(cls): if isinstance(cls, CustomBuilder): cls = cls.result_class_or_fn - else: - if get_origin(cls) is Annotated: - annotated, *annotations = get_args(cls) - return f"{_serialize_dep(annotated)}{repr(annotations)}" + elif get_origin(cls) is Annotated: + annotated, *annotations = get_args(cls) + return f"{_serialize_dep(annotated)}{annotations!r}" return get_fq_class_name(cls) class ScrapyPoetRequestFingerprinter: - IGNORED_UNANNOTATED_DEPS = { # These dependencies are tools for page objects that should have no # bearing on the request itself. @@ -89,10 +91,8 @@ def __init__(self, crawler: Crawler) -> None: ), crawler, ) - self._callback_cache: Dict[Callable, Optional[bytes]] = {} - self._request_cache: "WeakKeyDictionary[Request, bytes]" = ( - WeakKeyDictionary() - ) + self._callback_cache: dict[Callable, bytes | None] = {} + self._request_cache: WeakKeyDictionary[Request, bytes] = WeakKeyDictionary() self._crawler: Crawler = crawler self._saw_unserializable_page_params = False @@ -107,7 +107,7 @@ def _injector(self): "been configured in the DOWNLOADER_MIDDLEWARES setting?" ) - def _get_deps(self, request: Request) -> Optional[List[str]]: + def _get_deps(self, request: Request) -> list[str] | None: """Return a JSON-serializable structure that uniquely identifies the dependencies requested by the request, or None if dependency injection is not required.""" @@ -117,7 +117,7 @@ def _get_deps(self, request: Request) -> Optional[List[str]]: return None return sorted([_serialize_dep(cls) for cls in deps]) - def get_deps_key(self, request: Request) -> Optional[bytes]: + def get_deps_key(self, request: Request) -> bytes | None: """Return a JSON array as bytes that uniquely identifies the dependencies requested through scrapy-poet injection that could impact the request, or None if there are no such dependencies.""" @@ -134,7 +134,7 @@ def get_deps_key(self, request: Request) -> Optional[bytes]: self._callback_cache[callback] = deps_key return self._callback_cache[callback] - def serialize_page_params(self, request: Request) -> Optional[bytes]: + def serialize_page_params(self, request: Request) -> bytes | None: """Return a JSON object as bytes that represents the page params, or None if there are no page params or they are not JSON-serializable.""" @@ -175,5 +175,5 @@ def fingerprint(self, request: Request) -> bytes: if serialized_page_params is not None: fingerprint += serialized_page_params - self._request_cache[request] = hashlib.sha1(fingerprint).digest() + self._request_cache[request] = hashlib.sha1(fingerprint).digest() # noqa: S324 return self._request_cache[request] diff --git a/scrapy_poet/api.py b/scrapy_poet/api.py index 6efbf113..facc7fa6 100644 --- a/scrapy_poet/api.py +++ b/scrapy_poet/api.py @@ -1,5 +1,5 @@ from inspect import iscoroutinefunction -from typing import Callable, Optional, Type +from typing import Callable, Optional from scrapy.http import Request, Response from web_poet.pages import ItemPage @@ -31,7 +31,7 @@ def __init__(self, url: str, request: Optional[Request] = None): super().__init__(url=url, request=request) -def callback_for(page_or_item_cls: Type) -> Callable: +def callback_for(page_or_item_cls: type) -> Callable: """Create a callback for an :class:`web_poet.ItemPage ` subclass or an item class. @@ -116,11 +116,11 @@ def parse(self, response): # a dict of named arguments after our injectable. if issubclass(page_or_item_cls, ItemPage): - def parse(*args, page: page_or_item_cls, **kwargs): # type: ignore - yield page.to_item() # type: ignore + def parse(*args, page: page_or_item_cls, **kwargs): # type: ignore[valid-type] + yield page.to_item() # type: ignore[attr-defined] - async def async_parse(*args, page: page_or_item_cls, **kwargs): # type: ignore - yield await page.to_item() # type: ignore + async def async_parse(*args, page: page_or_item_cls, **kwargs): # type: ignore[valid-type] + yield await page.to_item() # type: ignore[attr-defined] if iscoroutinefunction(page_or_item_cls.to_item): setattr(async_parse, _CALLBACK_FOR_MARKER, True) @@ -128,7 +128,7 @@ async def async_parse(*args, page: page_or_item_cls, **kwargs): # type: ignore else: - def parse(*args, item: page_or_item_cls, **kwargs): # type:ignore + def parse(*args, item: page_or_item_cls, **kwargs): # type:ignore[valid-type,misc] yield item setattr(parse, _CALLBACK_FOR_MARKER, True) diff --git a/scrapy_poet/cache.py b/scrapy_poet/cache.py index f97852e3..9d0105fb 100644 --- a/scrapy_poet/cache.py +++ b/scrapy_poet/cache.py @@ -1,11 +1,15 @@ +from __future__ import annotations + import abc -import os import pickle from pathlib import Path -from typing import Any, Union +from typing import TYPE_CHECKING, Any from web_poet.serialization.api import SerializedData, SerializedDataFileStorage +if TYPE_CHECKING: + import os + class _Cache(abc.ABC): @abc.abstractmethod @@ -26,20 +30,18 @@ class SerializedDataCache(_Cache): `web_poet.serialization.SerializedDataFileStorage` """ - def __init__(self, directory: Union[str, os.PathLike]) -> None: + def __init__(self, directory: str | os.PathLike) -> None: self.directory = Path(directory) def __getitem__(self, fingerprint: str) -> SerializedData: storage = SerializedDataFileStorage(self._get_directory_path(fingerprint)) try: serialized_data = storage.read() - except FileNotFoundError: - raise KeyError(f"Fingerprint '{fingerprint}' not found in cache") + except FileNotFoundError as ex: + raise KeyError(f"Fingerprint '{fingerprint}' not found in cache") from ex return serialized_data - def __setitem__( - self, fingerprint: str, value: Union[SerializedData, Exception] - ) -> None: + def __setitem__(self, fingerprint: str, value: SerializedData | Exception) -> None: if isinstance(value, Exception): self.write_exception(fingerprint, value) else: diff --git a/scrapy_poet/commands.py b/scrapy_poet/commands.py index d11d3244..6a241d89 100644 --- a/scrapy_poet/commands.py +++ b/scrapy_poet/commands.py @@ -2,7 +2,7 @@ import logging import sys from pathlib import Path -from typing import Optional, Type +from typing import Optional import andi import scrapy @@ -49,7 +49,7 @@ def build_instances_from_providers( for cls, value in instances.items(): metadata = getattr(cls, "__metadata__", None) if metadata: - value = AnnotatedInstance(value, metadata) + value = AnnotatedInstance(value, metadata) # noqa: PLW2901 saved_dependencies.append(value) return instances @@ -65,10 +65,10 @@ def __init__(self, crawler: Crawler) -> None: def spider_for( - injectable: Type[ItemPage], + injectable: type[ItemPage], url: str, - base_spider: Optional[Type[scrapy.Spider]] = None, -) -> Type[scrapy.Spider]: + base_spider: Optional[type[scrapy.Spider]] = None, +) -> type[scrapy.Spider]: if base_spider is None: base_spider = scrapy.Spider @@ -81,7 +81,7 @@ def __init__(self, name=None, **kwargs): self.start_requests = lambda: [scrapy.Request(url, self.cb, meta=meta)] async def cb(self, response: DummyResponse, page: injectable): # type: ignore[valid-type] - global frozen_time + global frozen_time # noqa: PLW0603 frozen_time = datetime.datetime.now(datetime.timezone.utc).replace( microsecond=0 ) @@ -107,7 +107,7 @@ def short_desc(self): def run(self, args, opts): if len(args) < 2: - raise UsageError() + raise UsageError type_name = args[0] url = args[1] @@ -119,9 +119,9 @@ def run(self, args, opts): if not issubclass(cls, ItemPage): raise UsageError(f"Error: {type_name} is not a descendant of ItemPage") - self.settings["DOWNLOADER_MIDDLEWARES"][ - "scrapy_poet.InjectionMiddleware" - ] = None + self.settings["DOWNLOADER_MIDDLEWARES"]["scrapy_poet.InjectionMiddleware"] = ( + None + ) self.settings["DOWNLOADER_MIDDLEWARES"][ "scrapy_poet.downloadermiddlewares.InjectionMiddleware" ] = None diff --git a/scrapy_poet/downloadermiddlewares.py b/scrapy_poet/downloadermiddlewares.py index 25488de0..d5c1d2f6 100644 --- a/scrapy_poet/downloadermiddlewares.py +++ b/scrapy_poet/downloadermiddlewares.py @@ -3,15 +3,14 @@ are executed. """ +from __future__ import annotations + import inspect import logging import warnings -from typing import Generator, Optional, Type, TypeVar, Union +from typing import TYPE_CHECKING -from scrapy import Spider -from scrapy.crawler import Crawler from scrapy.downloadermiddlewares.stats import DownloaderStats -from scrapy.http import Request, Response from twisted.internet.defer import Deferred, inlineCallbacks from web_poet import RulesRegistry from web_poet.exceptions import Retry @@ -29,13 +28,23 @@ ) from .utils import create_registry_instance, is_min_scrapy_version +if TYPE_CHECKING: + from collections.abc import Generator + + from scrapy import Spider + from scrapy.crawler import Crawler + from scrapy.http import Request, Response + + # typing.Self requires Python 3.11 + from typing_extensions import Self + logger = logging.getLogger(__name__) class DownloaderStatsMiddleware(DownloaderStats): def process_response( self, request: Request, response: Response, spider: Spider - ) -> Union[Request, Response]: + ) -> Request | Response: if isinstance(response, DummyResponse): return response return super().process_response(request, response, spider) @@ -51,8 +60,6 @@ def process_response( StatsProvider: 1000, } -InjectionMiddlewareTV = TypeVar("InjectionMiddlewareTV", bound="InjectionMiddleware") - class InjectionMiddleware: """This is a Downloader Middleware that's supposed to: @@ -72,15 +79,10 @@ def __init__(self, crawler: Crawler) -> None: ) @classmethod - def from_crawler( - cls: Type[InjectionMiddlewareTV], crawler: Crawler - ) -> InjectionMiddlewareTV: - o = cls(crawler) - return o - - def process_request( - self, request: Request, spider: Spider - ) -> Optional[DummyResponse]: + def from_crawler(cls, crawler: Crawler) -> Self: + return cls(crawler) + + def process_request(self, request: Request, spider: Spider) -> DummyResponse | None: """This method checks if the request is really needed and if its download could be skipped by trying to infer if a :class:`scrapy.http.Response` is going to be used by the callback or a Page Input. @@ -125,15 +127,12 @@ def _skip_dependency_creation(self, request: Request, spider: Spider) -> bool: return False # Skip if providers are needed. - if self.injector.discover_callback_providers(request): - return True - - return False + return bool(self.injector.discover_callback_providers(request)) @inlineCallbacks def process_response( self, request: Request, response: Response, spider: Spider - ) -> Generator[Deferred, object, Union[Response, Request]]: + ) -> Generator[Deferred, object, Response | Request]: """This method fills :attr:`scrapy.Request.cb_kwargs ` with instances for the required Page Objects found in the callback signature. @@ -164,7 +163,9 @@ def process_response( except Retry as exception: # Needed for Twisted < 21.2.0. See the discussion thread linked below: # https://github.com/scrapinghub/scrapy-poet/pull/129#discussion_r1102693967 - from scrapy.downloadermiddlewares.retry import get_retry_request + from scrapy.downloadermiddlewares.retry import ( # noqa: PLC0415 + get_retry_request, + ) reason = str(exception) or "page_object_retry" new_request_or_none = get_retry_request( diff --git a/scrapy_poet/injection.py b/scrapy_poet/injection.py index 769f5ac3..74e3c242 100644 --- a/scrapy_poet/injection.py +++ b/scrapy_poet/injection.py @@ -1,22 +1,11 @@ import functools import inspect import logging -import os import pprint import warnings -from typing import ( - Any, - Callable, - Dict, - Iterable, - List, - Mapping, - Optional, - Set, - Type, - cast, - get_type_hints, -) +from collections.abc import Iterable, Mapping +from pathlib import Path +from typing import Any, Callable, Optional, cast, get_type_hints from weakref import WeakKeyDictionary import andi @@ -62,8 +51,6 @@ class DynamicDeps(dict): values with keys being dependency types. """ - pass - class Injector: """ @@ -84,7 +71,7 @@ def __init__( self.load_providers(default_providers) self.init_cache() - def load_providers(self, default_providers: Optional[Mapping] = None): # noqa: D102 + def load_providers(self, default_providers: Optional[Mapping] = None): providers_dict = { **(default_providers or {}), **self.crawler.settings.getdict("SCRAPY_POET_PROVIDERS"), @@ -103,14 +90,14 @@ def load_providers(self, default_providers: Optional[Mapping] = None): # noqa: self.providers ) - def init_cache(self): # noqa: D102 + def init_cache(self): self.cache = {} cache_path = self.crawler.settings.get("SCRAPY_POET_CACHE") # SCRAPY_POET_CACHE: True if cache_path and isinstance(cache_path, bool): - cache_path = os.path.join( - get_scrapy_data_path(createdir=True), "scrapy-poet-cache" + cache_path = str( + Path(get_scrapy_data_path(createdir=True), "scrapy-poet-cache") ) # SCRAPY_POET_CACHE: @@ -126,11 +113,11 @@ def init_cache(self): # noqa: D102 # This is different from the cache above as it only stores instances as long # as the request exists. This is useful for latter providers to re-use the # already built instances by earlier providers. - self.weak_cache: WeakKeyDictionary[Request, Dict] = WeakKeyDictionary() + self.weak_cache: WeakKeyDictionary[Request, dict] = WeakKeyDictionary() def available_dependencies_for_providers( self, request: Request, response: Response - ): # noqa: D102 + ): deps = { Crawler: self.crawler, Spider: self.spider, @@ -144,7 +131,7 @@ def available_dependencies_for_providers( def discover_callback_providers( self, request: Request - ) -> Set[PageObjectInputProvider]: + ) -> set[PageObjectInputProvider]: """Discover the providers that are required to fulfil the callback dependencies""" plan = self.build_plan(request) result = set() @@ -193,18 +180,18 @@ def _get_custom_builder( on the registry and also supports filling :class:`.DynamicDeps`. """ - @functools.lru_cache(maxsize=None) # to minimize the registry queries + @functools.cache # to minimize the registry queries def mapping_fn(dep_cls: Callable) -> Optional[Callable]: # building DynamicDeps if dep_cls is DynamicDeps: dynamic_types = request.meta.get("inject", []) if not dynamic_types: - return lambda: {} + return dict return self._get_dynamic_deps_factory(dynamic_types) # building items from pages - page_object_cls: Optional[Type[ItemPage]] = self.registry.page_cls_for_item( - request.url, cast(type, dep_cls) + page_object_cls: Optional[type[ItemPage]] = self.registry.page_cls_for_item( + request.url, cast("type", dep_cls) ) if not page_object_cls: return None @@ -236,7 +223,7 @@ def _get_dynamic_deps_factory_text( @staticmethod def _get_dynamic_deps_factory( - dynamic_types: List[type], + dynamic_types: list[type], ) -> Callable[..., DynamicDeps]: """Return a function that creates a :class:`.DynamicDeps` instance from its args. @@ -245,15 +232,15 @@ def _get_dynamic_deps_factory( corresponding args. It has correct type hints so that it can be used as an ``andi`` custom builder. """ - type_names: List[str] = [] + type_names: list[str] = [] for type_ in dynamic_types: - type_ = cast(type, strip_annotated(type_)) - if not isinstance(type_, type): + type_stripped = cast("type", strip_annotated(type_)) + if not isinstance(type_stripped, type): raise TypeError(f"Expected a dynamic dependency type, got {type_!r}") - type_names.append(type_.__name__) + type_names.append(type_stripped.__name__) txt = Injector._get_dynamic_deps_factory_text(type_names) - ns: Dict[str, Any] = {} - exec(txt, globals(), ns) + ns: dict[str, Any] = {} + exec(txt, globals(), ns) # noqa: S102 return ns["__create_fn__"](*dynamic_types) @inlineCallbacks @@ -274,8 +261,8 @@ def build_instances( # following the andi plan. assert self.crawler.stats for cls, kwargs_spec in plan.dependencies: - if cls not in instances.keys(): - result_cls: type = cast(type, cls) + if cls not in instances: + result_cls: type = cast("type", cls) if isinstance(cls, andi.CustomBuilder): result_cls = cls.result_class_or_fn instances[result_cls] = yield deferred_from_coro( @@ -297,12 +284,12 @@ def build_instances_from_providers( ): """Build dependencies handled by registered providers""" assert self.crawler.stats - instances: Dict[Callable, Any] = {} + instances: dict[Callable, Any] = {} scrapy_provided_dependencies = self.available_dependencies_for_providers( request, response ) dependencies_set = {cls for cls, _ in plan.dependencies} - objs: List[Any] + objs: list[Any] for provider in self.providers: provided_classes = { cls for cls in dependencies_set if provider.is_provided(cls) @@ -360,11 +347,11 @@ def build_instances_from_providers( self.crawler.stats.inc_value("poet/cache/firsthand") raise - objs_by_type: Dict[Callable, Any] = {} + objs_by_type: dict[Callable, Any] = {} for obj in objs: if isinstance(obj, AnnotatedInstance): cls = obj.get_annotated_cls() - obj = obj.result + obj = obj.result # noqa: PLW2901 else: cls = type(obj) objs_by_type[cls] = obj @@ -411,7 +398,7 @@ def check_all_providers_are_callable(providers): def is_class_provided_by_any_provider_fn( - providers: List[PageObjectInputProvider], + providers: list[PageObjectInputProvider], ) -> Callable[[Callable], bool]: """ Return a function of type ``Callable[[Type], bool]`` that return @@ -419,15 +406,12 @@ def is_class_provided_by_any_provider_fn( The ``is_provided`` method from each provider is used. """ - callables: List[Callable[[Callable], bool]] = [] - for provider in providers: - callables.append(provider.is_provided) + callables: list[Callable[[Callable], bool]] = [ + provider.is_provided for provider in providers + ] def is_provided_fn(type_: Callable) -> bool: - for is_provided in callables: - if is_provided(type_): - return True - return False + return any(is_provided(type_) for is_provided in callables) return is_provided_fn @@ -480,7 +464,8 @@ def is_callback_requiring_scrapy_response( "annotated with scrapy_poet.DummyResponse (or its subclasses), " "we're assuming this isn't intended and would simply ignore " "this annotation.\n\n" - "See the Pitfalls doc for more info." + "See the Pitfalls doc for more info.", + stacklevel=1, ) return True @@ -519,7 +504,7 @@ def is_provider_requiring_scrapy_response(provider): def get_injector_for_testing( providers: Mapping, - additional_settings: Optional[Dict] = None, + additional_settings: Optional[dict] = None, registry: Optional[RulesRegistry] = None, ) -> Injector: """ @@ -542,7 +527,7 @@ class MySpider(Spider): def get_response_for_testing( - callback: Callable, meta: Optional[Dict[str, Any]] = None + callback: Callable, meta: Optional[dict[str, Any]] = None ) -> Response: """ Return a :class:`scrapy.http.Response` with fake content with the configured @@ -561,9 +546,6 @@ def get_response_for_testing(

The best chocolate ever

- """.encode( - "utf-8" - ) + """.encode() request = Request(url, callback=callback, meta=meta) - response = Response(url, 200, None, html, request=request) - return response + return Response(url, 200, None, html, request=request) diff --git a/scrapy_poet/injection_errors.py b/scrapy_poet/injection_errors.py index 01e4e926..697ed692 100644 --- a/scrapy_poet/injection_errors.py +++ b/scrapy_poet/injection_errors.py @@ -22,5 +22,3 @@ class ProviderDependencyDeadlockError(InjectionError): - Page object named "ChickenPage" require "EggPage" as a dependency. - Page object named "EggPage" require "ChickenPage" as a dependency. """ - - pass diff --git a/scrapy_poet/page_input_providers.py b/scrapy_poet/page_input_providers.py index 9d404250..077340c8 100644 --- a/scrapy_poet/page_input_providers.py +++ b/scrapy_poet/page_input_providers.py @@ -9,7 +9,7 @@ for example, from scrapy-playwright or from an API for automatic extraction. """ -from typing import Any, Callable, ClassVar, FrozenSet, Set, Union +from typing import Any, Callable, ClassVar, Set, Union from scrapy import Request from scrapy.crawler import Crawler @@ -95,7 +95,7 @@ def __call__(self, to_provide, response: Response): is provided by this provider. """ - provided_classes: Union[Set[Callable], Callable[[Callable], bool]] + provided_classes: Union[set[Callable], Callable[[Callable], bool]] name: ClassVar[str] = "" # It must be a unique name. Used by the cache mechanism def is_provided(self, type_: Callable) -> bool: @@ -103,15 +103,14 @@ def is_provided(self, type_: Callable) -> bool: Return ``True`` if the given type is provided by this provider based on the value of the attribute ``provided_classes`` """ - if isinstance(self.provided_classes, (Set, FrozenSet)): + if isinstance(self.provided_classes, (set, frozenset)): return type_ in self.provided_classes - elif callable(self.provided_classes): + if callable(self.provided_classes): return self.provided_classes(type_) - else: - raise MalformedProvidedClassesError( - f"Unexpected type {type_!r} for 'provided_classes' attribute of" - f"{self!r}. Expected either 'set' or 'callable'" - ) + raise MalformedProvidedClassesError( + f"Unexpected type {type_!r} for 'provided_classes' attribute of" + f"{self!r}. Expected either 'set' or 'callable'" + ) # FIXME: Can't import the Injector as class annotation due to circular dep. def __init__(self, injector): @@ -243,10 +242,10 @@ def __init__(self, stats): self._stats = stats self._prefix = "poet/stats/" - def set(self, key: str, value: Any) -> None: # noqa: D102 + def set(self, key: str, value: Any) -> None: self._stats.set_value(f"{self._prefix}{key}", value) - def inc(self, key: str, value: StatNum = 1) -> None: # noqa: D102 + def inc(self, key: str, value: StatNum = 1) -> None: self._stats.inc_value(f"{self._prefix}{key}", value) diff --git a/scrapy_poet/spidermiddlewares.py b/scrapy_poet/spidermiddlewares.py index 8f7a1fda..d70b44c2 100644 --- a/scrapy_poet/spidermiddlewares.py +++ b/scrapy_poet/spidermiddlewares.py @@ -1,9 +1,13 @@ -from typing import List, Optional +from __future__ import annotations + +from typing import TYPE_CHECKING -from scrapy import Spider -from scrapy.http import Request, Response from web_poet.exceptions import Retry +if TYPE_CHECKING: + from scrapy import Spider + from scrapy.http import Request, Response + class RetryMiddleware: """Captures :exc:`web_poet.exceptions.Retry` exceptions from spider @@ -14,10 +18,12 @@ def process_spider_exception( response: Response, exception: BaseException, spider: Spider, - ) -> Optional[List[Request]]: + ) -> list[Request] | None: # Needed for Twisted < 21.2.0. See the discussion thread linked below: # https://github.com/scrapinghub/scrapy-poet/pull/129#discussion_r1102693967 - from scrapy.downloadermiddlewares.retry import get_retry_request + from scrapy.downloadermiddlewares.retry import ( # noqa: PLC0415 + get_retry_request, + ) if not isinstance(exception, Retry): return None diff --git a/scrapy_poet/utils/__init__.py b/scrapy_poet/utils/__init__.py index fa7c0372..99273f32 100644 --- a/scrapy_poet/utils/__init__.py +++ b/scrapy_poet/utils/__init__.py @@ -1,6 +1,5 @@ -import os from functools import lru_cache -from typing import Type +from pathlib import Path from packaging.version import Version from scrapy import __version__ as SCRAPY_VERSION @@ -31,7 +30,7 @@ def get_scrapy_data_path(createdir: bool = True, default_dir: str = ".scrapy") - # which does too many things. path = project_data_dir() if inside_project() else default_dir if createdir: - os.makedirs(path, exist_ok=True) + Path(path).mkdir(exist_ok=True, parents=True) return path @@ -84,13 +83,13 @@ def http_response_to_scrapy_response(response: HttpResponse) -> HtmlResponse: ) -def create_registry_instance(cls: Type, crawler: Crawler): +def create_registry_instance(cls: type, crawler: Crawler): for module in crawler.settings.getlist("SCRAPY_POET_DISCOVER", []): consume_modules(module) rules = crawler.settings.getlist("SCRAPY_POET_RULES", default_registry.get_rules()) return cls(rules=rules) -@lru_cache() +@lru_cache def is_min_scrapy_version(version: str) -> bool: return Version(SCRAPY_VERSION) >= Version(version) diff --git a/scrapy_poet/utils/mockserver.py b/scrapy_poet/utils/mockserver.py index 8d4203e3..24a363d3 100644 --- a/scrapy_poet/utils/mockserver.py +++ b/scrapy_poet/utils/mockserver.py @@ -16,15 +16,15 @@ def get_ephemeral_port(): class MockServer: def __init__(self, resource, port=None, pythonpath=None): - self.resource = "{0}.{1}".format(resource.__module__, resource.__name__) + self.resource = f"{resource.__module__}.{resource.__name__}" self.proc = None host = socket.gethostbyname(socket.gethostname()) self.port = port or get_ephemeral_port() - self.root_url = "http://%s:%d" % (host, self.port) + self.root_url = f"http://{host}:{self.port}" self.pythonpath = pythonpath or "" def __enter__(self): - self.proc = Popen( + self.proc = Popen( # noqa: S603 [ sys.executable, "-u", @@ -60,11 +60,7 @@ def main(): def print_listening(): host = http_port.getHost() - print( - "Mock server {0} running at http://{1}:{2}".format( - resource, host.host, host.port - ) - ) + print(f"Mock server {resource} running at http://{host.host}:{host.port}") reactor.callWhenRunning(print_listening) reactor.run() diff --git a/scrapy_poet/utils/testing.py b/scrapy_poet/utils/testing.py index e0cab1de..ac894ac7 100644 --- a/scrapy_poet/utils/testing.py +++ b/scrapy_poet/utils/testing.py @@ -1,6 +1,6 @@ +import contextlib import json from inspect import isasyncgenfunction -from typing import Dict from warnings import warn from scrapy import Spider, signals @@ -22,7 +22,7 @@ class HtmlResource(Resource): isLeaf = True content_type = "text/html" html = "" - extra_headers: Dict[str, str] = {} + extra_headers: dict[str, str] = {} status_code = 200 def render_GET(self, request): @@ -106,7 +106,6 @@ def render_GET(self, request): class ProductHtml(HtmlResource): - html = """
@@ -189,10 +188,8 @@ def setup_crawler_engine(crawler: Crawler): crawler.crawling = True crawler.spider = crawler._create_spider() crawler.settings.frozen = False - try: + with contextlib.suppress(AttributeError): # Scrapy < 2.10 crawler._apply_settings() - except AttributeError: - pass # Scrapy < 2.10 crawler.engine = crawler._create_engine() handler = get_download_handler(crawler, "https") @@ -248,15 +245,15 @@ def _get_test_settings(): }, } try: - import scrapy.addons # noqa: F401 + import scrapy.addons # noqa: F401,PLC0415 except ImportError: settings["DOWNLOADER_MIDDLEWARES"]["scrapy_poet.InjectionMiddleware"] = 543 settings["DOWNLOADER_MIDDLEWARES"][ "scrapy.downloadermiddlewares.stats.DownloaderStats" ] = None - settings["DOWNLOADER_MIDDLEWARES"][ - "scrapy_poet.DownloaderStatsMiddleware" - ] = 850 + settings["DOWNLOADER_MIDDLEWARES"]["scrapy_poet.DownloaderStatsMiddleware"] = ( + 850 + ) settings["REQUEST_FINGERPRINTER_CLASS"] = ScrapyPoetRequestFingerprinter settings["SPIDER_MIDDLEWARES"] = { "scrapy_poet.RetryMiddleware": 275, @@ -266,7 +263,7 @@ def _get_test_settings(): "scrapy_poet.Addon": 300, } try: - from scrapy.utils.test import get_reactor_settings + from scrapy.utils.test import get_reactor_settings # noqa: PLC0415 settings.update(get_reactor_settings()) except ImportError: @@ -300,7 +297,7 @@ async def parse(*args, **kwargs): yield x except Exception as e: yield {"exception": e} - raise CloseSpider("Exception in callback detected") + raise CloseSpider("Exception in callback detected") from e # Mimic type annotations parse.__annotations__ = callback.__annotations__ diff --git a/setup.py b/setup.py index f9e2b55c..53ceb1b0 100755 --- a/setup.py +++ b/setup.py @@ -1,15 +1,12 @@ -from os.path import dirname, join +from pathlib import Path from setuptools import find_packages, setup -with open(join(dirname(__file__), "scrapy_poet/VERSION"), "rb") as f: - version = f.read().decode("ascii").strip() - setup( name="scrapy-poet", - version=version, + version=Path("scrapy_poet", "VERSION").read_text().strip(), description="Page Object pattern for Scrapy", - long_description=open("README.rst").read(), + long_description=Path("README.rst").read_text(encoding="utf-8"), long_description_content_type="text/x-rst", author="Mikhail Korobov", author_email="kmike84@gmail.com", diff --git a/tests/conftest.py b/tests/conftest.py index b66fd233..698e9918 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,6 +3,6 @@ from scrapy_poet.utils.testing import _get_test_settings -@pytest.fixture() +@pytest.fixture def settings(): return _get_test_settings() diff --git a/tests/test_cache.py b/tests/test_cache.py index 5248b7ac..216d03e8 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -10,29 +10,28 @@ @inlineCallbacks def test_cache_no_errors(caplog) -> None: - with TemporaryDirectory() as cache_dir: - with MockServer(EchoResource) as server: + with TemporaryDirectory() as cache_dir, MockServer(EchoResource) as server: - class Page(WebPage): - @field - async def url(self): - return self.response.url + class Page(WebPage): + @field + async def url(self): + return self.response.url - class CacheSpider(Spider): - name = "cache" + class CacheSpider(Spider): + name = "cache" - custom_settings = { - **_get_test_settings(), - "SCRAPY_POET_CACHE": cache_dir, - } + custom_settings = { + **_get_test_settings(), + "SCRAPY_POET_CACHE": cache_dir, + } - def start_requests(self): - yield Request(server.root_url, callback=self.parse_url) + def start_requests(self): + yield Request(server.root_url, callback=self.parse_url) - async def parse_url(self, response, page: Page): - await page.to_item() + async def parse_url(self, response, page: Page): + await page.to_item() - crawler = make_crawler(CacheSpider, {}) - yield crawler.crawl() + crawler = make_crawler(CacheSpider, {}) + yield crawler.crawl() assert all(record.levelname != "ERROR" for record in caplog.records) diff --git a/tests/test_callback_for.py b/tests/test_callback_for.py index 61f3112b..a32b4c63 100644 --- a/tests/test_callback_for.py +++ b/tests/test_callback_for.py @@ -22,14 +22,12 @@ def to_item(self): class MySpider(scrapy.Spider): - name = "my_spider" parse_item = callback_for(FakeItemPage) parse_web = callback_for(FakeWebPage) class MySpiderAsync(scrapy.Spider): - name = "my_spider_async" parse_item = callback_for(FakeItemPageAsync) @@ -110,7 +108,7 @@ def test_inline_callback(): spider = MySpider() cb = callback_for(FakeItemPage) request = scrapy.Request("http://example.com/", callback=cb) - with pytest.raises(ValueError) as exc: + with pytest.raises(ValueError, match="is not an instance method in:") as exc: request.to_dict(spider=spider) msg = f"Function {cb} is not an instance method in: {spider}" @@ -122,7 +120,7 @@ def test_inline_callback_async(): spider = MySpiderAsync() cb = callback_for(FakeItemPageAsync) request = scrapy.Request("http://example.com/", callback=cb) - with pytest.raises(ValueError) as exc: + with pytest.raises(ValueError, match="is not an instance method in:") as exc: request.to_dict(spider=spider) msg = f"Function {cb} is not an instance method in: {spider}" diff --git a/tests/test_commands.py b/tests/test_commands.py index e82386e0..9632f750 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -24,9 +24,9 @@ def call_scrapy_command(cwd: str, *args: str, run_module: bool = True) -> None: with tempfile.TemporaryFile() as out: if run_module: - args = (sys.executable, "-m", "scrapy.cmdline") + args + args = (sys.executable, "-m", "scrapy.cmdline", *args) else: - args = ("scrapy",) + args + args = ("scrapy", *args) status = subprocess.call(args, stdout=out, stderr=out, cwd=cwd) out.seek(0) assert status == 0, out.read().decode() @@ -42,7 +42,7 @@ def __init__(self): def _get_pythonpath() -> str: # needed for mockserver to find CustomResource as the pytester fixture changes the working directory - return str(Path(os.path.dirname(__file__)).parent) + return str(Path(__file__).parent.parent) def test_savefixture(pytester) -> None: diff --git a/tests/test_downloader.py b/tests/test_downloader.py index 7e2630f1..42a93830 100644 --- a/tests/test_downloader.py +++ b/tests/test_downloader.py @@ -1,7 +1,8 @@ import sys import warnings +from collections.abc import Sequence from functools import partial -from typing import Any, Callable, List, Optional, Sequence, Set +from typing import TYPE_CHECKING, Any, Callable, Optional, Set from unittest import mock from urllib.parse import urlparse @@ -14,7 +15,6 @@ from scrapy import Request, Spider from scrapy.crawler import Crawler from scrapy.exceptions import IgnoreRequest -from scrapy.http import Response from scrapy.utils.defer import maybe_deferred_to_future from web_poet import BrowserResponse, HttpClient from web_poet.exceptions import HttpError, HttpRequestError, HttpResponseError @@ -36,6 +36,9 @@ make_crawler, ) +if TYPE_CHECKING: + from scrapy.http import Response + @pytest.fixture def scrapy_downloader() -> Callable: @@ -70,7 +73,6 @@ async def test_scrapy_poet_downloader(fake_http_response) -> None: with mock.patch( "scrapy_poet.downloader.maybe_deferred_to_future", new_callable=mock.AsyncMock ) as mock_dtf: - mock_dtf.return_value = fake_http_response mock_downloader = mock.MagicMock(return_value=mock.AsyncMock) @@ -133,7 +135,7 @@ async def test_scrapy_poet_downloader_head_redirect(fake_http_response) -> None: await scrapy_downloader(req) - args, kwargs = mock_downloader.call_args + args, _ = mock_downloader.call_args scrapy_request = args[0] assert scrapy_request.meta.get("dont_redirect") is True @@ -516,7 +518,7 @@ class BaseSpider(Spider): # See: https://github.com/scrapinghub/scrapy-poet/issues/48 def _assert_warning_messages( - record, index: Optional[List] = None, not_existing: bool = False + record, index: Optional[list] = None, not_existing: bool = False ): index = index or [0, 1] @@ -575,7 +577,7 @@ def parse(self, response: DummyResponse): # type: ignore[override] crawler = make_crawler(TestSpider) - with pytest.warns(UserWarning) as record: + with pytest.warns(UserWarning) as record: # noqa: PT030 yield crawler.crawl() _assert_warning_messages(record, index=[0]) @@ -606,7 +608,7 @@ def parse(self, response: scrapy.http.Response): crawler = make_crawler(TestSpider) - with pytest.warns(UserWarning) as record: + with pytest.warns(UserWarning) as record: # noqa: PT030 yield crawler.crawl() _assert_warning_messages(record, not_existing=True) @@ -667,7 +669,7 @@ def parse(self, response: DummyResponse, page: BasicPage): # type: ignore[overr crawler = make_crawler(TestSpider) - with pytest.warns(UserWarning) as record: + with pytest.warns(UserWarning) as record: # noqa: PT030 yield crawler.crawl() _assert_warning_messages(record) @@ -710,7 +712,7 @@ def parse(self, response: DummyResponse, page: BasicPage): # type: ignore[overr crawler = make_crawler(TestSpider) - with pytest.warns(UserWarning) as record: + with pytest.warns(UserWarning) as record: # noqa: PT030 yield crawler.crawl() _assert_warning_messages(record, index=[0]) @@ -750,7 +752,7 @@ def parse( # type: ignore[override] crawler = make_crawler(TestSpider) - with pytest.warns(UserWarning) as record: + with pytest.warns(UserWarning) as record: # noqa: PT030 yield crawler.crawl() _assert_warning_messages(record) @@ -787,7 +789,7 @@ def parse(self, response: DummyResponse): # type: ignore[override] crawler = make_crawler(TestSpider) - with pytest.warns(UserWarning) as record: + with pytest.warns(UserWarning) as record: # noqa: PT030 yield crawler.crawl() _assert_warning_messages(record, not_existing=True) @@ -819,7 +821,7 @@ def parse(self, response: DummyResponse, page: BasicPage): # type: ignore[overr crawler = make_crawler(TestSpider) - with pytest.warns(UserWarning) as record: + with pytest.warns(UserWarning) as record: # noqa: PT030 yield crawler.crawl() _assert_warning_messages(record, not_existing=True) diff --git a/tests/test_injection.py b/tests/test_injection.py index 696dadd5..a3ac2ff3 100644 --- a/tests/test_injection.py +++ b/tests/test_injection.py @@ -1,6 +1,7 @@ import re import shutil -from typing import Annotated, Any, Callable, Dict, Generator, Optional +from collections.abc import Generator +from typing import Annotated, Any, Callable, Optional import andi import attr @@ -225,7 +226,7 @@ def callback( def test_build_instances_from_providers_unexpected_return(self): class WrongProvider(get_provider({Cls1})): def __call__(self, to_provide): - return super().__call__(to_provide) + [Cls2()] + return [*super().__call__(to_provide), Cls2()] injector = get_injector_for_testing({WrongProvider: 0}) @@ -297,9 +298,9 @@ def callback( def _assert_instances( injector: Injector, callback: Callable, - expected_instances: Dict[type, Any], - expected_kwargs: Dict[str, Any], - reqmeta: Optional[Dict[str, Any]] = None, + expected_instances: dict[type, Any], + expected_kwargs: dict[str, Any], + reqmeta: Optional[dict[str, Any]] = None, ) -> Generator[Any, Any, None]: response = get_response_for_testing(callback, meta=reqmeta) assert response.request @@ -683,7 +684,7 @@ class PricePO(ItemPage, ResponseShortcutsMixin): response: Html # type: ignore[assignment] def to_item(self): - return dict(price=float(self.css(".price::text").get()), currency="€") + return {"price": float(self.css(".price::text").get()), "currency": "€"} @attr.s(auto_attribs=True) @@ -711,8 +712,8 @@ async def to_item(self): class TestInjectorStats: @pytest.mark.parametrize( - "cb_args, expected", - ( + ("cb_args", "expected"), + [ ( {"price_po": PricePO, "rate_po": EurDollarRate}, { @@ -738,7 +739,7 @@ class TestInjectorStats: {"item": TestItem}, set(), # there must be no stats as TestItem is not in the registry ), - ), + ], ) @inlineCallbacks def test_stats(self, cb_args, expected, injector): @@ -927,9 +928,11 @@ def callback(response: DummyResponse, arg_price: Price, arg_name: Name): # > response.request = Request.replace(response.request, url="http://willfail.page") - with pytest.raises(ValueError): - plan = injector.build_plan(response.request) - instances = yield from injector.build_instances_from_providers( + plan = injector.build_plan(response.request) + with pytest.raises( + ValueError, match=r"\(http://willfail.page\) is not example.com" + ): + yield from injector.build_instances_from_providers( response.request, response, plan ) assert injector.weak_cache.get(response.request) is None @@ -953,9 +956,9 @@ def callback(response: DummyResponse, arg_price: Price, arg_name: Name): # If caching errors is disabled, then KeyError should be raised. Error = ValueError if cache_errors else KeyError response.request = Request.replace(response.request, url="http://willfail.page") + plan = injector.build_plan(response.request) with pytest.raises(Error): - plan = injector.build_plan(response.request) - instances = yield from injector.build_instances_from_providers( + yield from injector.build_instances_from_providers( response.request, response, plan ) assert injector.weak_cache.get(response.request) is None diff --git a/tests/test_middleware.py b/tests/test_middleware.py index 9fea2eba..afd9cdee 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -3,7 +3,7 @@ import subprocess import sys from textwrap import dedent -from typing import Optional, Type, Union +from typing import Optional, Union import andi import attr @@ -30,7 +30,7 @@ ) -def spider_for(injectable: Type): +def spider_for(injectable: type): class InjectableSpider(scrapy.Spider): url = None custom_settings = { @@ -376,7 +376,7 @@ class UnressolvableProductPage(ProductPage): @inlineCallbacks def test_injection_failure(settings): configure_logging(settings) - items, url, crawler = yield crawl_items( + items, *_ = yield crawl_items( spider_for(UnressolvableProductPage), ProductHtml, settings ) assert items == [] @@ -408,14 +408,14 @@ def parse(self, response: DummyResponse): # type: ignore[override] @inlineCallbacks def test_skip_downloads(settings): - item, url, crawler = yield crawl_single_item(MySpider, ProductHtml, settings) + item, _, crawler = yield crawl_single_item(MySpider, ProductHtml, settings) assert isinstance(item["response"], Response) is True assert isinstance(item["response"], DummyResponse) is False assert crawler.stats.get_stats().get("downloader/request_count", 0) == 1 assert crawler.stats.get_stats().get("scrapy_poet/dummy_response_count", 0) == 0 assert crawler.stats.get_stats().get("downloader/response_count", 0) == 1 - item, url, crawler = yield crawl_single_item( + item, _, crawler = yield crawl_single_item( SkipDownloadSpider, ProductHtml, settings ) assert isinstance(item["response"], Response) is True @@ -544,7 +544,7 @@ def test_skip_download_request_url_page(settings): def test_scrapy_shell(tmp_path): try: - import scrapy.addons # noqa: F401 + import scrapy.addons # noqa: F401,PLC0415 except ImportError: settings = """ DOWNLOADER_MIDDLEWARES = { @@ -593,4 +593,4 @@ def test_scrapy_shell(tmp_path): pytest.fail("Command took too much time to complete") assert b"Using DummyResponse instead of downloading" not in err - assert b"{}" in out # noqa: P103 + assert b"{}" in out diff --git a/tests/test_page_input_providers.py b/tests/test_page_input_providers.py index 9590f0e9..8f4778af 100644 --- a/tests/test_page_input_providers.py +++ b/tests/test_page_input_providers.py @@ -20,8 +20,8 @@ class Provider(PageObjectInputProvider): def test_is_provided_on_function(self): class Provider(PageObjectInputProvider): @staticmethod - def provided_classes(cls): - return issubclass(cls, str) + def provided_classes(type_): + return issubclass(type_, str) class SubStr(str): pass diff --git a/tests/test_providers.py b/tests/test_providers.py index 2c36298e..b8b9b40b 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -1,4 +1,4 @@ -from typing import Any, Callable, List, Set, Type +from typing import Any, Callable, Set from unittest import mock import attr @@ -66,7 +66,7 @@ def __call__( self, to_provide, response: scrapy.http.Response, spider: scrapy.Spider ): assert isinstance(spider, scrapy.Spider) - ret: List[Any] = [] + ret: list[Any] = [] if Price in to_provide: price = response.css(".price::text").get() assert price is not None @@ -82,7 +82,7 @@ class NameHtmlDataProvider(PageObjectInputProvider): def __call__(self, to_provide, response: scrapy.http.Response, settings: Settings): assert isinstance(settings, Settings) - ret: List[Any] = [] + ret: list[Any] = [] if Name in to_provide: name = response.css(".name::text").get() assert name is not None @@ -104,7 +104,7 @@ def fingerprint(self, to_provide: Set[Callable], request: Request) -> str: def _serialize(o: dep_cls) -> SerializedLeafData: # type: ignore[valid-type] return {"txt": attr.astuple(o)[0].encode()} - def _deserialize(cls: Type[dep_cls], data: SerializedLeafData) -> dep_cls: # type: ignore[valid-type] + def _deserialize(cls: type[dep_cls], data: SerializedLeafData) -> dep_cls: # type: ignore[valid-type] return cls(data["txt"].decode()) # type: ignore[misc] register_serialization(_serialize, _deserialize) diff --git a/tests/test_request_fingerprinter.py b/tests/test_request_fingerprinter.py index d39ceccc..a7678d6b 100644 --- a/tests/test_request_fingerprinter.py +++ b/tests/test_request_fingerprinter.py @@ -28,6 +28,7 @@ PageParams, RequestUrl, ResponseUrl, + RulesRegistry, Stats, WebPage, ) @@ -59,7 +60,7 @@ def get_crawler(spider_cls=None, settings=None, ensure_providers_for=None): if spider_cls is not None: kwargs["spider_cls"] = spider_cls - ensure_providers_for = ensure_providers_for or tuple() + ensure_providers_for = ensure_providers_for or () if ensure_providers_for: dummy_providers = get_dummy_providers(*ensure_providers_for) if dummy_providers: @@ -204,7 +205,7 @@ async def parse_dummy(self, response: DummyResponse, web: WebPage): @pytest.mark.parametrize( "input_cls", - ( + [ HttpClient, HttpRequest, HttpRequestBody, @@ -212,7 +213,7 @@ async def parse_dummy(self, response: DummyResponse, web: WebPage): PageParams, RequestUrl, Stats, - ), + ], ) def test_ignored_unannotated_page_inputs(input_cls): """These web-poet page input classes, unless annotated, cannot have any @@ -270,7 +271,7 @@ async def parse_input(self, response, some_input: input_cls): @pytest.mark.parametrize( - "input_cls_a, input_cls_b", + ("input_cls_a", "input_cls_b"), (tuple(combination) for combination in combinations(FINGERPRINTING_INPUTS, 2)), ) def test_fingerprinting_unannotated_page_input_combinations(input_cls_a, input_cls_b): @@ -360,14 +361,14 @@ def test_page_params(caplog): @pytest.mark.parametrize( "meta", - ( + [ {}, {"page_params": None}, {"page_params": {}}, {"foo": "bar"}, {"foo": "bar", "page_params": None}, {"foo": "bar", "page_params": {}}, - ), + ], ) def test_meta(meta): crawler = get_crawler() @@ -507,9 +508,6 @@ async def parse_page(self, response, page: WebPage): def test_item(settings): """Test that fingerprinting works even for items.""" - from scrapy import Request, Spider - from web_poet import ItemPage, RulesRegistry - registry = RulesRegistry() class MyItem: diff --git a/tests/test_response_required_logic.py b/tests/test_response_required_logic.py index b774cc3d..836fd7ad 100644 --- a/tests/test_response_required_logic.py +++ b/tests/test_response_required_logic.py @@ -1,5 +1,5 @@ import warnings -from typing import Any, Dict +from typing import Any import attr import pytest @@ -27,12 +27,12 @@ @attr.s(auto_attribs=True) class DummyProductResponse: - data: Dict[str, Any] + data: dict[str, Any] @attr.s(auto_attribs=True) class FakeProductResponse: - data: Dict[str, Any] + data: dict[str, Any] class DummyProductProvider(PageObjectInputProvider): @@ -82,8 +82,7 @@ def url(self): return self.response.data["product"]["url"] def to_item(self): - product = self.response.data["product"] - return product + return self.response.data["product"] @attr.s(auto_attribs=True) @@ -95,8 +94,7 @@ def url(self): return self.response.data["product"]["url"] def to_item(self): - product = self.response.data["product"] - return product + return self.response.data["product"] class BookPage(WebPage): @@ -296,14 +294,6 @@ def cb(_: Any) -> Any: # See: https://github.com/scrapinghub/scrapy-poet/issues/48 request.callback = None - expected_warning = ( - "A request has been encountered with callback=None which " - "defaults to the parse() method. If the parse() method is " - "annotated with scrapy_poet.DummyResponse (or its subclasses), " - "we're assuming this isn't intended and would simply ignore " - "this annotation.\n\n" - "See the Pitfalls doc for more info." - ) assert is_callback_requiring_scrapy_response(spider.parse, request.callback) is True assert ( @@ -347,11 +337,13 @@ def cb(_: Any) -> Any: spider.parse8, spider.parse10, ): - with pytest.warns(UserWarning) as record: + with pytest.warns( + UserWarning, + match=r"encountered with callback=None which defaults to the parse\(\) method", + ): assert ( is_callback_requiring_scrapy_response(method, request.callback) is True # type: ignore[arg-type] ) - assert expected_warning in str(record.list[0].message) @pytest.mark.skipif( diff --git a/tests/test_retries.py b/tests/test_retries.py index 8031eb22..3eaa1484 100644 --- a/tests/test_retries.py +++ b/tests/test_retries.py @@ -1,5 +1,5 @@ from collections import deque -from typing import Any, List +from typing import Any from urllib.parse import urlparse import attrs @@ -18,7 +18,7 @@ class BaseSpider(Spider): name = "test_spider" -def _assert_all_unique_instances(instances: List[Any]): +def _assert_all_unique_instances(instances: list[Any]): assert len({id(instance) for instance in instances}) == len(instances) @@ -70,7 +70,6 @@ class TestItem: foo: str with MockServer(EchoResource) as server: - netloc = urlparse(server.root_url).netloc @registry.handle_urls(netloc) diff --git a/tests/test_scrapy_dependencies.py b/tests/test_scrapy_dependencies.py index 186d9765..0d68481f 100644 --- a/tests/test_scrapy_dependencies.py +++ b/tests/test_scrapy_dependencies.py @@ -23,7 +23,6 @@ class PageData: scrapy_class: str class PageDataProvider(PageObjectInputProvider): - provided_classes = {PageData} def __call__(self, to_provide, obj: scrapy_class): # type: ignore[valid-type] @@ -31,7 +30,6 @@ def __call__(self, to_provide, obj: scrapy_class): # type: ignore[valid-type] @attr.s(auto_attribs=True) class Page(WebPage): - page_data: PageData def to_item(self): @@ -40,7 +38,6 @@ def to_item(self): } class MySpider(Spider): - name = "my_spider" url = None custom_settings = { @@ -56,7 +53,7 @@ def start_requests(self): def parse(self, response, page: Page): return page.to_item() - item, url, crawler = yield crawl_single_item(MySpider, ProductHtml, settings) + item, *_ = yield crawl_single_item(MySpider, ProductHtml, settings) assert item["scrapy_class"] == scrapy_class.__name__ @@ -67,7 +64,6 @@ def test_scrapy_dependencies_on_page_objects(scrapy_class, settings) -> None: @attr.s(auto_attribs=True) class Page(WebPage): - scrapy_obj: scrapy_class # type: ignore[valid-type] def to_item(self): @@ -76,7 +72,6 @@ def to_item(self): } class MySpider(Spider): - name = "my_spider" url = None @@ -86,5 +81,5 @@ def start_requests(self): def parse(self, response, page: Page): return page.to_item() - items, url, crawler = yield crawl_items(MySpider, ProductHtml, settings) + items, *_ = yield crawl_items(MySpider, ProductHtml, settings) assert not items diff --git a/tests/test_utils.py b/tests/test_utils.py index e4e5ff54..5bed841c 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,5 +1,6 @@ -from pathlib import PosixPath +from pathlib import Path from unittest import mock +from unittest.mock import MagicMock import pytest from scrapy import Spider @@ -17,26 +18,29 @@ ) -@mock.patch("scrapy_poet.utils.os.makedirs") +@mock.patch("scrapy_poet.utils.Path", autospec=True) @mock.patch("scrapy_poet.utils.inside_project") -def test_get_scrapy_data_path(mock_inside_project, mock_makedirs, tmp_path): +def test_get_scrapy_data_path( + mock_inside_project: MagicMock, mock_path: MagicMock, tmp_path: Path +) -> None: mock_inside_project.return_value = False path = tmp_path / "test_dir" - result = get_scrapy_data_path(createdir=True, default_dir=path) + result = get_scrapy_data_path(createdir=True, default_dir=str(path)) - assert isinstance(result, PosixPath) - assert str(result) # should be non-empty + assert isinstance(result, str) + assert result # should be non-empty mock_inside_project.assert_called_once() - mock_makedirs.assert_called_once() - mock_makedirs.assert_called_with(path, exist_ok=True) + mock_path.assert_called_once_with(str(path)) + mock_path_obj = mock_path.return_value + mock_path_obj.mkdir.assert_called_once_with(exist_ok=True, parents=True) @pytest.mark.parametrize( - "http_request,kwargs,scrapy_request", - ( + ("http_request", "kwargs", "scrapy_request"), + [ ( HttpRequest("https://example.com"), {}, @@ -84,7 +88,7 @@ def test_get_scrapy_data_path(mock_inside_project, mock_makedirs, tmp_path): {}, Request("https://example.com", callback=NO_CALLBACK, body=b"a"), ), - ), + ], ) def test_http_request_to_scrapy_request(http_request, kwargs, scrapy_request): result = http_request_to_scrapy_request(http_request, **kwargs) @@ -104,8 +108,8 @@ def test_http_request_to_scrapy_request(http_request, kwargs, scrapy_request): @pytest.mark.parametrize( - "scrapy_response,http_response", - ( + ("scrapy_response", "http_response"), + [ ( Response("https://example.com"), HttpResponse("https://example.com", body=b"", status=200), @@ -169,7 +173,7 @@ def test_http_request_to_scrapy_request(http_request, kwargs, scrapy_request): "https://example.com", body=b"a", status=200, encoding="utf-8" ), ), - ), + ], ) def test_scrapy_response_to_http_response(scrapy_response, http_response): result = scrapy_response_to_http_response(scrapy_response) @@ -181,8 +185,8 @@ def test_scrapy_response_to_http_response(scrapy_response, http_response): @pytest.mark.parametrize( - "scrapy_response,http_response", - ( + ("scrapy_response", "http_response"), + [ ( Response("https://example.com"), HttpResponse("https://example.com", body=b"", status=200), @@ -246,7 +250,7 @@ def test_scrapy_response_to_http_response(scrapy_response, http_response): "https://example.com", body=b"a", status=200, encoding="utf-8" ), ), - ), + ], ) def test_http_response_to_scrapy_response(scrapy_response, http_response): result = http_response_to_scrapy_response(http_response) diff --git a/tests/test_web_poet_rules.py b/tests/test_web_poet_rules.py index 3a217562..640928f9 100644 --- a/tests/test_web_poet_rules.py +++ b/tests/test_web_poet_rules.py @@ -10,7 +10,7 @@ import socket import warnings from collections import defaultdict -from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Type +from typing import Any, Callable, Optional, Set import attrs import pytest @@ -55,7 +55,7 @@ def rules_settings() -> dict: return settings -def spider_for(injectable: Type): +def spider_for(injectable: type): class InjectableSpider(scrapy.Spider): url = None @@ -77,7 +77,7 @@ class PageObjectCounterMixin: additional requests used to produce the item. """ - instances: Dict[Type, Any] = defaultdict(list) + instances: dict[type, Any] = defaultdict(list) to_item_call_count = 0 def __attrs_pre_init__(self): @@ -89,7 +89,7 @@ def assert_instance_count(cls, count, type_): @classmethod def clear(cls): - for po_cls in cls.instances.keys(): + for po_cls in cls.instances: po_cls.to_item_call_count = 0 cls.instances = defaultdict(list) @@ -100,15 +100,15 @@ async def to_item(self) -> ItemT: @inlineCallbacks def crawl_item_and_deps( - page_object, override_settings: Optional[Dict] = None -) -> Tuple[Any, Any]: + page_object, override_settings: Optional[dict] = None +) -> tuple[Any, Any]: """Helper function to easily return the item and injected dependencies from a simulated Scrapy callback which asks for either of these dependencies: - page object - item class """ - settings = {**rules_settings(), **{"SCRAPY_POET_PROVIDERS": DEFAULT_PROVIDERS}} + settings = {**rules_settings(), "SCRAPY_POET_PROVIDERS": DEFAULT_PROVIDERS} if override_settings: settings.update(override_settings) @@ -118,7 +118,7 @@ def crawl_item_and_deps( return item, crawler.spider.collected_response_deps -def assert_deps(deps: List[Dict[str, Any]], expected: Dict[str, Any], size: int = 1): +def assert_deps(deps: list[dict[str, Any]], expected: dict[str, Any], size: int = 1): """Helper for easily checking the instances of the ``deps`` returned by ``crawl_item_and_deps()``. @@ -137,12 +137,10 @@ def assert_deps(deps: List[Dict[str, Any]], expected: Dict[str, Any], size: int def assert_warning_tokens(caught_warnings, expected_warning_tokens): - results = [] - for expected in expected_warning_tokens: - results.append( - any(expected in str(warning.message) for warning in caught_warnings) - ) - assert all(results) + assert all( + any(expected in str(warning.message) for warning in caught_warnings) + for expected in expected_warning_tokens + ) @inlineCallbacks @@ -1483,9 +1481,7 @@ def test_page_object_returning_item_which_is_also_a_dep() -> None: by the corresponding PO. """ - settings = { - "SCRAPY_POET_PROVIDERS": {**DEFAULT_PROVIDERS, **{MobiusProvider: 1000}} - } + settings = {"SCRAPY_POET_PROVIDERS": {**DEFAULT_PROVIDERS, MobiusProvider: 1000}} # item from 'to_return' item, deps = yield crawl_item_and_deps(Mobius, override_settings=settings) assert item == Mobius(name="(modified) mobius from MobiusProvider") @@ -1557,9 +1553,7 @@ def test_page_object_returning_item_which_is_also_a_dep_2() -> None: The PO with the higher priority should be the one returning the item. """ - settings = { - "SCRAPY_POET_PROVIDERS": {**DEFAULT_PROVIDERS, **{KangarooProvider: 1000}} - } + settings = {"SCRAPY_POET_PROVIDERS": {**DEFAULT_PROVIDERS, KangarooProvider: 1000}} # item from 'to_return' item, deps = yield crawl_item_and_deps(Kangaroo, override_settings=settings) assert item == Kangaroo(name="(modified by Joey) data from KangarooProvider")