Skip to content

Commit

Permalink
Don't inherit from immutabledict
Browse files Browse the repository at this point in the history
They recently changed from using __init__() to __new__(), which breaks
super().__init__() calls. Instead of switching to __new__ or trying to
temporarily support both, it's simpler to just not inherit from it in
the first place.
  • Loading branch information
dseomn committed Nov 30, 2023
1 parent b597bac commit bf59312
Show file tree
Hide file tree
Showing 8 changed files with 190 additions and 122 deletions.
31 changes: 17 additions & 14 deletions rock_paper_sand/justwatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@
"""Code that uses JustWatch's API."""

import collections
from collections.abc import Collection, Generator, Iterable, Mapping, Set
from collections.abc import Collection, Generator, Iterable, Mapping, Sequence, Set # fmt: skip
import contextlib
import dataclasses
import datetime
from typing import Any

import dateutil.parser
import immutabledict
import requests
import requests_cache

Expand Down Expand Up @@ -279,14 +280,18 @@ class _Offer:
comments: tuple[str, ...]


class _OfferResultExtra(media_filter.ResultExtra):
PROVIDER = "justwatch.provider"
COMMENTS = "justwatch._comments"
PUBLIC_KEYS = (PROVIDER,)
_PROVIDER_KEY = "justwatch.provider"

def human_readable(self) -> str | None:
"""See base class."""
return f"{self[self.PROVIDER]} ({', '.join(self[self.COMMENTS])})"

def _offer_result_extra(
*,
provider: str,
comments: Sequence[str],
) -> media_filter.ResultExtra:
return media_filter.ResultExtra(
human_readable=f"{provider} ({', '.join(comments)})",
data=immutabledict.immutabledict({_PROVIDER_KEY: provider}),
)


@dataclasses.dataclass(kw_only=True)
Expand Down Expand Up @@ -318,11 +323,9 @@ def to_extra_information(self) -> Set[media_filter.ResultExtra]:
*offer.comments,
)
extra_information.add(
_OfferResultExtra(
{
_OfferResultExtra.PROVIDER: offer.provider_name,
_OfferResultExtra.COMMENTS: comments,
}
_offer_result_extra(
provider=offer.provider_name,
comments=comments,
)
)
return extra_information
Expand Down Expand Up @@ -365,7 +368,7 @@ def valid_extra_keys(self) -> Set[str]:
"""See base class."""
keys: set[str] = set()
if self._should_check_availability():
keys.update(_OfferResultExtra.PUBLIC_KEYS)
keys.add(_PROVIDER_KEY)
return keys

def _iter_leaf_nodes(
Expand Down
22 changes: 13 additions & 9 deletions rock_paper_sand/justwatch_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,9 @@ def _offer(
def _offer_extra(
provider: str,
comments: tuple[str, ...],
) -> justwatch._OfferResultExtra:
return justwatch._OfferResultExtra( # pylint: disable=protected-access
{
justwatch._OfferResultExtra.PROVIDER: provider, # pylint: disable=protected-access
justwatch._OfferResultExtra.COMMENTS: comments, # pylint: disable=protected-access
}
) -> media_filter.ResultExtra:
return justwatch._offer_result_extra( # pylint: disable=protected-access
provider=provider, comments=comments
)


Expand Down Expand Up @@ -716,7 +713,7 @@ def test_exception_note(self) -> None:
error.exception.__notes__,
)

def test_extra_human_readable(self) -> None:
def test_extra(self) -> None:
self._mock_api.get_node.return_value = {
"offers": [
_offer(
Expand Down Expand Up @@ -746,8 +743,15 @@ def test_extra_human_readable(self) -> None:
)

self.assertEqual(
{f"Foo+ (bar, until {_TIME_IN_FUTURE_1})"},
{extra.human_readable() for extra in result.extra},
{
media_filter.ResultExtra(
human_readable=f"Foo+ (bar, until {_TIME_IN_FUTURE_1})",
data=immutabledict.immutabledict(
{"justwatch.provider": "Foo+"}
),
)
},
result.extra,
)


Expand Down
28 changes: 10 additions & 18 deletions rock_paper_sand/media_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"""Filters for media items."""

import abc
from collections.abc import Callable, Hashable, Iterable, Set
from collections.abc import Callable, Hashable, Iterable, Mapping, Set
import dataclasses
import datetime
import functools
Expand Down Expand Up @@ -59,27 +59,19 @@ def cache_key(self) -> Hashable:
return (self.item.id, self.now)


class ResultExtra(immutabledict.immutabledict[str, Any]):
@dataclasses.dataclass(frozen=True, kw_only=True)
class ResultExtra:
"""Extra information about a filter result.
Keys should generally be scoped with dots. E.g., the justwatch filter should
use keys like "justwatch.provider" instead of just "provider".
Attributes:
human_readable: Human-readable description of the extra info, or None.
data: Data for grouping or use by parent filters. Keys should generally
be scoped with dots. E.g., the justwatch filter should use keys like
"justwatch.provider" instead of just "provider".
"""

def human_readable(self) -> str | None:
"""Returns a human-readable description of the extra info, or None."""
return None


class ResultExtraString(ResultExtra):
"""Simple ResultExtra implementation for strings with no additional data."""

def __init__(self, human_readable: str, /) -> None:
super().__init__({"_result_extra_string": human_readable})

def human_readable(self) -> str | None:
"""See base class."""
return self["_result_extra_string"]
human_readable: str | None = None
data: Mapping[str, Any] = immutabledict.immutabledict()


@dataclasses.dataclass(frozen=True)
Expand Down
80 changes: 51 additions & 29 deletions rock_paper_sand/media_filter_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ def __init__(self, extra: Set[media_filter.ResultExtra]) -> None:

def valid_extra_keys(self) -> Set[str]:
"""See base class."""
return frozenset(itertools.chain.from_iterable(self._extra))
return frozenset(
itertools.chain.from_iterable(extra.data for extra in self._extra)
)

def filter_implementation(
self, request: media_filter.FilterRequest
Expand All @@ -54,8 +56,12 @@ def filter_implementation(
return media_filter.FilterResult(True, extra=self._extra)


_EXTRA_1 = media_filter.ResultExtra(test="extra-1")
_EXTRA_2 = media_filter.ResultExtra(test="extra-2")
_EXTRA_1 = media_filter.ResultExtra(
data=immutabledict.immutabledict(test="extra-1"),
)
_EXTRA_2 = media_filter.ResultExtra(
data=immutabledict.immutabledict(test="extra-2"),
)


class MediaFilterTest(parameterized.TestCase):
Expand All @@ -65,26 +71,6 @@ def test_filter_request_cache_key(self) -> None:
)
self.assertEqual((request.item.id, request.now), request.cache_key())

@parameterized.named_parameters(
dict(
testcase_name="default",
result_extra=media_filter.ResultExtra(foo="bar"),
human_readable=None,
),
dict(
testcase_name="string",
result_extra=media_filter.ResultExtraString("some-string"),
human_readable="some-string",
),
)
def test_result_extra_human_readable(
self,
*,
result_extra: media_filter.ResultExtra,
human_readable: str | None,
) -> None:
self.assertEqual(human_readable, result_extra.human_readable())

@parameterized.named_parameters(
dict(
testcase_name="all",
Expand Down Expand Up @@ -290,24 +276,48 @@ def test_basic_filter(
dict(
testcase_name="ref",
filter_by_name=dict(
foo=_ExtraInfoFilter({media_filter.ResultExtra(some_key="bar")})
foo=_ExtraInfoFilter(
{
media_filter.ResultExtra(
data=immutabledict.immutabledict(some_key="bar")
)
}
)
),
filter_config={"ref": "foo"},
valid_extra_keys={"some_key"},
),
dict(
testcase_name="not",
filter_by_name=dict(
foo=_ExtraInfoFilter({media_filter.ResultExtra(some_key="bar")})
foo=_ExtraInfoFilter(
{
media_filter.ResultExtra(
data=immutabledict.immutabledict(some_key="bar")
)
}
)
),
filter_config={"not": {"ref": "foo"}},
valid_extra_keys={"some_key"},
),
dict(
testcase_name="and",
filter_by_name=dict(
foo=_ExtraInfoFilter({media_filter.ResultExtra(key_1="baz")}),
bar=_ExtraInfoFilter({media_filter.ResultExtra(key_2="baz")}),
foo=_ExtraInfoFilter(
{
media_filter.ResultExtra(
data=immutabledict.immutabledict(key_1="baz")
)
}
),
bar=_ExtraInfoFilter(
{
media_filter.ResultExtra(
data=immutabledict.immutabledict(key_2="baz")
)
}
),
),
filter_config={
"and": {"filters": [{"ref": "foo"}, {"ref": "bar"}]}
Expand All @@ -317,8 +327,20 @@ def test_basic_filter(
dict(
testcase_name="or",
filter_by_name=dict(
foo=_ExtraInfoFilter({media_filter.ResultExtra(key_1="baz")}),
bar=_ExtraInfoFilter({media_filter.ResultExtra(key_2="baz")}),
foo=_ExtraInfoFilter(
{
media_filter.ResultExtra(
data=immutabledict.immutabledict(key_1="baz")
)
}
),
bar=_ExtraInfoFilter(
{
media_filter.ResultExtra(
data=immutabledict.immutabledict(key_2="baz")
)
}
),
),
filter_config={"or": {"filters": [{"ref": "foo"}, {"ref": "bar"}]}},
valid_extra_keys={"key_1", "key_2"},
Expand Down
8 changes: 4 additions & 4 deletions rock_paper_sand/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ def _filter_media_item(
extra_information.append("parent did not match, but children did")
extra_information.extend(
sorted(
extra_str
extra.human_readable
for extra in item_result.extra
if (extra_str := extra.human_readable()) is not None
if extra.human_readable is not None
)
)
if extra_information:
Expand Down Expand Up @@ -180,9 +180,9 @@ def _generate_group_by(
if not item_result.matches:
continue
groups = {
extra[self.proto.group_by.key]
extra.data[self.proto.group_by.key]
for extra in item_result.extra
if self.proto.group_by.key in extra
if self.proto.group_by.key in extra.data
}
for group in groups:
results[group].append(item.fully_qualified_name)
Expand Down
17 changes: 14 additions & 3 deletions rock_paper_sand/report_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from absl.testing import absltest
from absl.testing import parameterized
from google.protobuf import json_format
import immutabledict

from rock_paper_sand import media_filter
from rock_paper_sand import media_item
Expand Down Expand Up @@ -59,7 +60,11 @@ def filter(
first_word, _, _ = request.item.proto.name.partition(" ")
return media_filter.FilterResult(
first_word != "no",
extra={media_filter.ResultExtra(first_word=first_word)},
extra={
media_filter.ResultExtra(
data=immutabledict.immutabledict(first_word=first_word)
)
},
)


Expand Down Expand Up @@ -281,12 +286,18 @@ def test_report_generate(
filter_registry = media_filter.Registry()
filter_registry.register(
"extra-without-str",
_ExtraInfoFilter({media_filter.ResultExtra(test="no-str")}),
_ExtraInfoFilter(
{
media_filter.ResultExtra(
data=immutabledict.immutabledict(test="no-str")
)
}
),
)
filter_registry.register(
"extra-with-str",
_ExtraInfoFilter(
{media_filter.ResultExtraString("example extra info")}
{media_filter.ResultExtra(human_readable="example extra info")}
),
)
filter_registry.register("first-word-is-not-no", _FirstWordIsNotNo())
Expand Down
18 changes: 11 additions & 7 deletions rock_paper_sand/wikidata.py
Original file line number Diff line number Diff line change
Expand Up @@ -810,8 +810,8 @@ def _related_item_result_extra(
) is not None:
item_description_parts.append(f"({description})")
item_description_parts.append(f"<{item_ref}>")
return media_filter.ResultExtraString(
f"{category}: {' '.join(item_description_parts)}"
return media_filter.ResultExtra(
human_readable=f"{category}: {' '.join(item_description_parts)}",
)

def _related_media(
Expand Down Expand Up @@ -904,15 +904,19 @@ def _related_media(
)
),
*(
media_filter.ResultExtraString(
"item in config file that's not related to "
f"{request.item.wikidata_item}: {item}"
media_filter.ResultExtra(
human_readable=(
"item in config file that's not related to "
f"{request.item.wikidata_item}: {item}"
),
)
for item in items_from_config - processed - loose
),
*(
media_filter.ResultExtraString(
f"item configured to be ignored, but not found: {item}"
media_filter.ResultExtra(
human_readable=(
f"item configured to be ignored, but not found: {item}"
),
)
for item in (
request.item.wikidata_ignore_items_recursive
Expand Down
Loading

0 comments on commit bf59312

Please sign in to comment.