From 2f48d2f2489f3ead38e2e352a75975b14e32f5d4 Mon Sep 17 00:00:00 2001 From: Bart Feenstra Date: Sun, 6 Oct 2024 19:41:48 +0100 Subject: [PATCH] Refactor betty.wikipedia's Image and Summary into dataclasses (#2082) --- betty/assertion/__init__.py | 3 + betty/cli/__init__.py | 3 +- betty/fetch/__init__.py | 5 +- betty/gramps/loader.py | 3 +- .../project/extension/cotton_candy/search.py | 2 +- betty/tests/coverage/test_coverage.py | 18 ++- betty/tests/wikipedia/test___init__.py | 48 ------- betty/wikipedia/__init__.py | 127 +++--------------- 8 files changed, 48 insertions(+), 161 deletions(-) diff --git a/betty/assertion/__init__.py b/betty/assertion/__init__.py index d69d76a56..0d5b73938 100644 --- a/betty/assertion/__init__.py +++ b/betty/assertion/__init__.py @@ -18,6 +18,7 @@ overload, cast, TypeAlias, + final, ) from betty.assertion.error import AssertionFailedGroup, AssertionFailed, Key, Index @@ -98,6 +99,7 @@ class _Field(Generic[_AssertionValueT, _AssertionReturnT]): assertion: Assertion[_AssertionValueT, _AssertionReturnT] | None = None +@final @dataclass(frozen=True) class RequiredField( Generic[_AssertionValueT, _AssertionReturnT], @@ -110,6 +112,7 @@ class RequiredField( pass # pragma: no cover +@final @dataclass(frozen=True) class OptionalField( Generic[_AssertionValueT, _AssertionReturnT], diff --git a/betty/cli/__init__.py b/betty/cli/__init__.py index db55e6f03..e368b434f 100644 --- a/betty/cli/__init__.py +++ b/betty/cli/__init__.py @@ -132,7 +132,8 @@ async def make_context( return ctx -@dataclass +@final +@dataclass(frozen=True) class ContextAppObject: """ The running Betty application and it localizer. diff --git a/betty/fetch/__init__.py b/betty/fetch/__init__.py index f887695c3..30c851bef 100644 --- a/betty/fetch/__init__.py +++ b/betty/fetch/__init__.py @@ -6,7 +6,7 @@ from dataclasses import dataclass from json import loads from pathlib import Path -from typing import Any +from typing import Any, final from multidict import CIMultiDict @@ -21,7 +21,8 @@ class FetchError(UserFacingError, RuntimeError): pass # pragma: no cover -@dataclass +@final +@dataclass(frozen=True) class FetchResponse: """ An HTTP response. diff --git a/betty/gramps/loader.py b/betty/gramps/loader.py index 0ed85fc70..705a127d0 100644 --- a/betty/gramps/loader.py +++ b/betty/gramps/loader.py @@ -13,7 +13,7 @@ from enum import Enum from logging import getLogger from pathlib import Path -from typing import Iterable, Any, IO, cast, TYPE_CHECKING, TypeVar, Generic +from typing import Iterable, Any, IO, cast, TYPE_CHECKING, TypeVar, Generic, final from xml.etree import ElementTree import aiofiles @@ -112,6 +112,7 @@ class GrampsEntityType(Enum): SOURCE = "source" +@final @dataclass(frozen=True) class GrampsEntityReference: """ diff --git a/betty/project/extension/cotton_candy/search.py b/betty/project/extension/cotton_candy/search.py index 92fc436c8..276668179 100644 --- a/betty/project/extension/cotton_candy/search.py +++ b/betty/project/extension/cotton_candy/search.py @@ -89,7 +89,7 @@ class _SourceIndexer(_EntityTypeIndexer[Source]): @final -@dataclass +@dataclass(frozen=True) class _Entry: text: set[str] result: str diff --git a/betty/tests/coverage/test_coverage.py b/betty/tests/coverage/test_coverage.py index 5afa182bd..48b47aed2 100644 --- a/betty/tests/coverage/test_coverage.py +++ b/betty/tests/coverage/test_coverage.py @@ -221,7 +221,13 @@ class TestKnownToBeMissing: # This is inherited from @dataclass. "__eq__": TestKnownToBeMissing, # This is inherited from @dataclass. + "__delattr__": TestKnownToBeMissing, + # This is inherited from @dataclass. + "__hash__": TestKnownToBeMissing, + # This is inherited from @dataclass. "__replace__": TestKnownToBeMissing, + # This is inherited from @dataclass. + "__setattr__": TestKnownToBeMissing, }, }, "betty/fetch/static.py": TestKnownToBeMissing, @@ -608,11 +614,21 @@ class TestKnownToBeMissing: }, "betty/warnings.py": TestKnownToBeMissing, "betty/wikipedia/__init__.py": { + # This is a dataclass. "Image": TestKnownToBeMissing, # This is an empty class. "NotAPageError": TestKnownToBeMissing, "Summary": { - "name": TestKnownToBeMissing, + # This is inherited from @dataclass. + "__eq__": TestKnownToBeMissing, + # This is inherited from @dataclass. + "__delattr__": TestKnownToBeMissing, + # This is inherited from @dataclass. + "__hash__": TestKnownToBeMissing, + # This is inherited from @dataclass. + "__replace__": TestKnownToBeMissing, + # This is inherited from @dataclass. + "__setattr__": TestKnownToBeMissing, }, }, } diff --git a/betty/tests/wikipedia/test___init__.py b/betty/tests/wikipedia/test___init__.py index 6ada329ad..83eee28a4 100644 --- a/betty/tests/wikipedia/test___init__.py +++ b/betty/tests/wikipedia/test___init__.py @@ -91,54 +91,6 @@ async def test_url(self) -> None: sut = Summary("nl", "Amsterdam", "Title for Amsterdam", "Content for Amsterdam") assert sut.url == "https://nl.wikipedia.org/wiki/Amsterdam" - async def test_title(self) -> None: - title = "Title for Amsterdam" - sut = Summary("nl", "Amsterdam", title, "Content for Amsterdam") - assert sut.title == title - - async def test_content(self) -> None: - content = "Content for Amsterdam" - sut = Summary("nl", "Amsterdam", "Title for Amsterdam", content) - assert sut.content == content - - @pytest.mark.parametrize( - ("expected", "left", "right"), - [ - ( - True, - Summary("en", "name", "title", "content"), - Summary("en", "name", "title", "content"), - ), - ( - False, - Summary("en", "name", "title", "content"), - Summary("nl", "name", "title", "content"), - ), - ( - False, - Summary("en", "name", "title", "content"), - Summary("en", "not-a-name", "title", "content"), - ), - ( - False, - Summary("en", "name", "title", "content"), - Summary("en", "name", "not-a-title", "content"), - ), - ( - False, - Summary("en", "name", "title", "content"), - Summary("en", "name", "title", "not-a-content"), - ), - ( - False, - Summary("en", "name", "title", "content"), - 123, - ), - ], - ) - async def test___eq__(self, expected: bool, left: Summary, right: object) -> None: - assert (left == right) is expected - class TestRetriever: @pytest.mark.parametrize( diff --git a/betty/wikipedia/__init__.py b/betty/wikipedia/__init__.py index d19457829..78cad2fef 100644 --- a/betty/wikipedia/__init__.py +++ b/betty/wikipedia/__init__.py @@ -8,17 +8,14 @@ import re from asyncio import gather from collections import defaultdict -from collections.abc import ( - Mapping, -) +from collections.abc import Mapping from contextlib import suppress, contextmanager +from dataclasses import dataclass from json import JSONDecodeError from pathlib import Path -from typing import cast, Any, TYPE_CHECKING +from typing import cast, Any, TYPE_CHECKING, final from urllib.parse import quote, urlparse -from geopy import Point - from betty.ancestry.file import File from betty.ancestry.file_reference import FileReference from betty.ancestry.has_file_references import HasFileReferences @@ -36,20 +33,15 @@ ) from betty.locale.error import LocaleError from betty.locale.localizable import plain -from betty.locale.localized import Localized from betty.media_type import MediaType from betty.media_type.media_types import HTML +from geopy import Point if TYPE_CHECKING: from betty.ancestry import Ancestry from betty.locale.localizer import LocalizerRepository from betty.fetch import Fetcher - from collections.abc import ( - Sequence, - MutableSequence, - MutableMapping, - Iterator, - ) + from collections.abc import Sequence, MutableSequence, MutableMapping, Iterator class NotAPageError(ValueError): @@ -70,117 +62,38 @@ def _parse_url(url: str) -> tuple[str, str]: return cast(tuple[str, str], match.groups()) -class Summary(Localized): +@final +@dataclass(frozen=True) +class Summary: """ A Wikipedia page summary. """ - def __init__(self, locale: str, name: str, title: str, content: str): - self._name = name - self._title = title - self._content = content - self._locale = locale - - def __eq__(self, other: object) -> bool: - if not isinstance(other, Summary): - return False - if self.name != other.name: - return False - if self.url != other.url: - return False - if self.title != other.title: - return False - if self.content != other.content: - return False - return True - - @property - def name(self) -> str: - """ - The page's machine name. - """ - return self._name + locale: str + name: str + title: str + content: str @property def url(self) -> str: """ The URL to the web page. """ - return f"https://{self.locale}.wikipedia.org/wiki/{self._name}" - - @property - def title(self) -> str: - """ - The page's human-readable title. - """ - return self._title - - @property - def content(self) -> str: - """ - The page's human-readable summary content. - """ - return self._content + return f"https://{self.locale}.wikipedia.org/wiki/{self.name}" +@final +@dataclass(frozen=True) class Image: """ An image from Wikimedia Commons. """ - def __init__( - self, - path: Path, - media_type: MediaType, - title: str, - wikimedia_commons_url: str, - name: str, - ): - self._path = path - self._media_type = media_type - self._title = title - self._wikimedia_commons_url = wikimedia_commons_url - self._name = name - - def __hash__(self) -> int: - return hash( - (self.path, self.media_type, self.title, self.wikimedia_commons_url) - ) - - @property - def path(self) -> Path: - """ - The path to the image on disk. - """ - return self._path - - @property - def media_type(self) -> MediaType: - """ - The image's media type. - """ - return self._media_type - - @property - def title(self) -> str: - """ - The human-readable image title. - """ - return self._title - - @property - def wikimedia_commons_url(self) -> str: - """ - The URL to the Wikimedia Commons web page for this image. - """ - return self._wikimedia_commons_url - - @property - def name(self) -> str: - """ - The image's file name. - """ - return self._name + path: Path + media_type: MediaType + title: str + wikimedia_commons_url: str + name: str class _Retriever: