From ffc5e7f7217780e01b023e1bbe89c4402bfb9196 Mon Sep 17 00:00:00 2001 From: Bart Feenstra Date: Mon, 6 May 2024 22:17:41 +0100 Subject: [PATCH] Deprecate betty.functools.walk() (#1459) --- betty/extension/cotton_candy/__init__.py | 5 +- .../templates/entity/featured--place.html.j2 | 2 +- .../templates/entity/page--person.html.j2 | 4 +- .../templates/entity/page--place.html.j2 | 2 +- .../templates/entity/page--source.html.j2 | 2 +- betty/functools.py | 5 ++ betty/model/ancestry.py | 27 +++++++- betty/privatizer.py | 3 +- betty/tests/model/test_ancestry.py | 58 ++++++++++++++++ betty/tests/test_functools.py | 67 ++++++++++--------- betty/tests/test_jinja2.py | 16 +++-- documentation/usage/templating/filters.rst | 1 - 12 files changed, 143 insertions(+), 49 deletions(-) diff --git a/betty/extension/cotton_candy/__init__.py b/betty/extension/cotton_candy/__init__.py index 536f41b90..39a08afa8 100644 --- a/betty/extension/cotton_candy/__init__.py +++ b/betty/extension/cotton_candy/__init__.py @@ -18,7 +18,6 @@ from betty.config import Configuration from betty.extension.cotton_candy.search import Index from betty.extension.webpack import Webpack, WebpackEntrypointProvider -from betty.functools import walk from betty.gui import GuiBuilder from betty.html import CssProvider from betty.jinja2 import ( @@ -427,9 +426,9 @@ def _person_timeline_events(person: Person, lifetime_threshold: int) -> Iterable is_public, ( # All ancestors. - *walk(person, "parents"), + *person.ancestors, # All descendants. - *walk(person, "children"), + *person.descendants, # All siblings. *person.siblings, ), diff --git a/betty/extension/cotton_candy/assets/templates/entity/featured--place.html.j2 b/betty/extension/cotton_candy/assets/templates/entity/featured--place.html.j2 index b2b15a386..5794b8b33 100644 --- a/betty/extension/cotton_candy/assets/templates/entity/featured--place.html.j2 +++ b/betty/extension/cotton_candy/assets/templates/entity/featured--place.html.j2 @@ -17,7 +17,7 @@
- {% set places = place | walk('encloses') | select('entity', 'Place') | list %} + {% set places = place.walk_encloses | map(attribute='encloses') | select('entity', 'Place') | list %} {% if place.coordinates %} {% set places = places + [place] %} {% endif %} diff --git a/betty/extension/cotton_candy/assets/templates/entity/page--person.html.j2 b/betty/extension/cotton_candy/assets/templates/entity/page--person.html.j2 index 76f725e36..8a92959cd 100644 --- a/betty/extension/cotton_candy/assets/templates/entity/page--person.html.j2 +++ b/betty/extension/cotton_candy/assets/templates/entity/page--person.html.j2 @@ -90,7 +90,7 @@ {%- endtrans -%} {% endif %}

- {% set ancestral_affiliation_names = person | walk('parents') | select('public') | map(attribute='names') | flatten | select('public') | map(attribute='affiliation') | reject('none') | unique | list | sort %} + {% set ancestral_affiliation_names = person.ancestors | select('public') | map(attribute='names') | flatten | select('public') | map(attribute='affiliation') | reject('none') | unique | list | sort %} {% if ancestral_affiliation_names | length > 0 %}

{%- trans -%} @@ -167,7 +167,7 @@

{% set ns = namespace(descendant_affiliation_names=[]) %} {% for per_parent_child in family_children %} - {% set ns.descendant_affiliation_names = ns.descendant_affiliation_names + (per_parent_child | walk('children') | list + [per_parent_child]) | select('public') | map(attribute='names') | flatten | select('public') | map(attribute='affiliation') | reject('none') | list %} + {% set ns.descendant_affiliation_names = ns.descendant_affiliation_names + (per_parent_child.descendants | list + [per_parent_child]) | select('public') | map(attribute='names') | flatten | select('public') | map(attribute='affiliation') | reject('none') | list %} {% endfor %} {% set ns.descendant_affiliation_names = ns.descendant_affiliation_names | unique | list | sort | list %} {% if ns.descendant_affiliation_names | length > 0 %} diff --git a/betty/extension/cotton_candy/assets/templates/entity/page--place.html.j2 b/betty/extension/cotton_candy/assets/templates/entity/page--place.html.j2 index 883efc7df..1fa61a814 100644 --- a/betty/extension/cotton_candy/assets/templates/entity/page--place.html.j2 +++ b/betty/extension/cotton_candy/assets/templates/entity/page--place.html.j2 @@ -4,7 +4,7 @@ {% block page_content %} {% include 'entity/meta--place.html.j2' %} - {% set places = place | walk('encloses') | select('entity', 'Place') | list %} + {% set places = place.walk_encloses | map(attribute='encloses') | select('entity', 'Place') | list %} {% if place.coordinates %} {% set places = places + [place] %} {% endif %} diff --git a/betty/extension/cotton_candy/assets/templates/entity/page--source.html.j2 b/betty/extension/cotton_candy/assets/templates/entity/page--source.html.j2 index 653a385e8..bde5274d4 100644 --- a/betty/extension/cotton_candy/assets/templates/entity/page--source.html.j2 +++ b/betty/extension/cotton_candy/assets/templates/entity/page--source.html.j2 @@ -7,7 +7,7 @@ {% include 'section-notes.html.j2' %} {% endwith %} - {% set sources = [source] + source | walk('contains') | select('public') | list %} + {% set sources = [source] + source.walk_contains | select('public') | list %} {% set have_files = sources %} {% set have_files = have_files + sources | map(attribute='citations') | flatten | list %} {% with files = have_files | select('public') | map(attribute='files') | flatten | unique %} diff --git a/betty/functools.py b/betty/functools.py index 65aecbbec..323160467 100644 --- a/betty/functools.py +++ b/betty/functools.py @@ -20,10 +20,15 @@ Awaitable, ) +from betty.warnings import deprecated + T = TypeVar("T") U = TypeVar("U") +@deprecated( + "This function is deprecated as of Betty 0.3.5, and will be removed in Betty 0.4.x. Instead, use a custom function, tailored to your data type." +) def walk(item: Any, attribute_name: str) -> Iterable[Any]: """ Walk over a graph of objects by following a single attribute. diff --git a/betty/model/ancestry.py b/betty/model/ancestry.py index f5627331c..f0c9ffbd1 100644 --- a/betty/model/ancestry.py +++ b/betty/model/ancestry.py @@ -4,7 +4,7 @@ from __future__ import annotations -from collections.abc import MutableSequence +from collections.abc import MutableSequence, Iterator from contextlib import suppress from enum import Enum from pathlib import Path @@ -792,6 +792,12 @@ def citations(self, citations: Iterable[Citation]) -> None: def citations(self) -> None: pass + @property + def walk_contains(self) -> Iterator[Source]: + for source in self.contains: + yield source + yield from source.contains + @classmethod def entity_type_label(cls) -> Str: return Str._("Source") @@ -1165,6 +1171,13 @@ def events(self, events: Iterable[Event]) -> None: def events(self) -> None: pass + @property + def walk_encloses(self) -> Iterator[Enclosure]: + for enclosure in self.encloses: + yield enclosure + if enclosure.encloses is not None: + yield from enclosure.encloses.walk_encloses + @classmethod def entity_type_label(cls) -> Str: return Str._("Place") @@ -1851,6 +1864,12 @@ def entity_type_label(cls) -> Str: def entity_type_label_plural(cls) -> Str: return Str._("People") + @property + def ancestors(self) -> Iterator[Person]: + for parent in self.parents: + yield parent + yield from parent.ancestors + @property def siblings(self) -> list[Person]: siblings = [] @@ -1860,6 +1879,12 @@ def siblings(self) -> list[Person]: siblings.append(sibling) return siblings + @property + def descendants(self) -> Iterator[Person]: + for child in self.children: + yield child + yield from child.descendants + @property def associated_files(self) -> Iterable[File]: files = [ diff --git a/betty/privatizer.py b/betty/privatizer.py index 203da1cae..212363df4 100644 --- a/betty/privatizer.py +++ b/betty/privatizer.py @@ -9,7 +9,6 @@ from datetime import datetime from typing import Iterator, TypeAlias, Any -from betty.functools import walk from betty.locale import DateRange, Date, Localizer from betty.model import Entity from betty.model.ancestry import ( @@ -199,7 +198,7 @@ def _determine_person_privacy(self, person: Person) -> None: return # If any descendant has any expired event, the person is considered not private. - for descendant in walk(person, "children"): + for descendant in person.descendants: if self.has_expired(descendant, 1): person.public = True return diff --git a/betty/tests/model/test_ancestry.py b/betty/tests/model/test_ancestry.py index 6b953194d..79c78a150 100644 --- a/betty/tests/model/test_ancestry.py +++ b/betty/tests/model/test_ancestry.py @@ -672,6 +672,16 @@ async def test_contains(self) -> None: sut.contains = [contains_source] # type: ignore[assignment] assert [contains_source] == list(sut.contains) + async def test_walk_contains_without_contains(self) -> None: + sut = Source() + assert [] == list(sut.walk_contains) + + async def test_walk_contains_with_contains(self) -> None: + sut = Source() + contains = Source(contained_by=sut) + contains_contains = Source(contained_by=contains) + assert [contains, contains_contains] == list(sut.walk_contains) + async def test_citations(self) -> None: sut = Source() assert [] == list(sut.citations) @@ -1246,6 +1256,30 @@ async def test_encloses(self) -> None: assert [] == list(sut.encloses) assert enclosure.enclosed_by is None + async def test_walk_encloses_without_encloses(self) -> None: + sut = Place( + id="P1", + names=[PlaceName(name="The Place")], + ) + assert [] == list(sut.walk_encloses) + + async def test_walk_encloses_with_encloses(self) -> None: + sut = Place( + id="P1", + names=[PlaceName(name="The Place")], + ) + encloses_place = Place( + id="P2", + names=[PlaceName(name="The Other Place")], + ) + encloses = Enclosure(encloses_place, sut) + encloses_encloses_place = Place( + id="P2", + names=[PlaceName(name="The Other Other Place")], + ) + encloses_encloses = Enclosure(encloses_encloses_place, encloses_place) + assert [encloses, encloses_encloses] == list(sut.walk_encloses) + async def test_id(self) -> None: place_id = "C1" sut = Place( @@ -1872,6 +1906,30 @@ async def test_siblings_with_multiple_common_parents(self) -> None: parent.children = [sut, sibling] # type: ignore[assignment] assert [sibling] == list(sut.siblings) + async def test_ancestors_without_parents(self) -> None: + sut = Person(id="person") + assert [] == list(sut.ancestors) + + async def test_ancestors_with_parent(self) -> None: + sut = Person(id="1") + parent = Person(id="3") + sut.parents.add(parent) + grandparent = Person(id="2") + parent.parents.add(grandparent) + assert [parent, grandparent] == list(sut.ancestors) + + async def test_descendants_without_parents(self) -> None: + sut = Person(id="person") + assert [] == list(sut.descendants) + + async def test_descendants_with_parent(self) -> None: + sut = Person(id="1") + child = Person(id="3") + sut.children.add(child) + grandchild = Person(id="2") + child.children.add(grandchild) + assert [child, grandchild] == list(sut.descendants) + async def test_associated_files(self) -> None: file1 = File(path=Path()) file2 = File(path=Path()) diff --git a/betty/tests/test_functools.py b/betty/tests/test_functools.py index 4363cc6a1..780602a6f 100644 --- a/betty/tests/test_functools.py +++ b/betty/tests/test_functools.py @@ -1,14 +1,15 @@ from collections.abc import Awaitable, Callable, Iterable -from typing import Any +from typing import Any, Self import pytest from betty.functools import walk, slice_to_range, Do +from betty.warnings import BettyDeprecationWarning class TestWalk: class _Item: - def __init__(self, child: "TestWalk._Item | Iterable[TestWalk._Item] | None"): + def __init__(self, child: Self | Iterable[Self] | None): self.child = child @pytest.mark.parametrize( @@ -21,44 +22,50 @@ def __init__(self, child: "TestWalk._Item | Iterable[TestWalk._Item] | None"): ], ) async def test_with_invalid_attribute(self, item: Any) -> None: - with pytest.raises(AttributeError): - list(walk(item, "invalid_attribute_name")) + with pytest.warns(BettyDeprecationWarning): + with pytest.raises(AttributeError): + list(walk(item, "invalid_attribute_name")) async def test_one_to_one_without_descendants(self) -> None: - item = self._Item(None) - actual = walk(item, "child") - expected: list[None] = [] - assert expected == list(actual) + with pytest.warns(BettyDeprecationWarning): + item = self._Item(None) + actual = walk(item, "child") + expected: list[None] = [] + assert expected == list(actual) async def test_one_to_one_with_descendants(self) -> None: - grandchild = self._Item(None) - child = self._Item(grandchild) - item = self._Item(child) - actual = walk(item, "child") - expected = [child, grandchild] - assert expected == list(actual) + with pytest.warns(BettyDeprecationWarning): + grandchild = self._Item(None) + child = self._Item(grandchild) + item = self._Item(child) + actual = walk(item, "child") + expected = [child, grandchild] + assert expected == list(actual) async def test_one_to_many_without_descendants(self) -> None: - item = self._Item([]) - actual = walk(item, "child") - expected: list[None] = [] - assert expected == list(actual) + with pytest.warns(BettyDeprecationWarning): + item = self._Item([]) + actual = walk(item, "child") + expected: list[None] = [] + assert expected == list(actual) async def test_with_one_to_many_descendants(self) -> None: - grandchild = self._Item([]) - child = self._Item([grandchild]) - item = self._Item([child]) - actual = walk(item, "child") - expected = [child, grandchild] - assert expected == list(actual) + with pytest.warns(BettyDeprecationWarning): + grandchild = self._Item([]) + child = self._Item([grandchild]) + item = self._Item([child]) + actual = walk(item, "child") + expected = [child, grandchild] + assert expected == list(actual) async def test_with_mixed_descendants(self) -> None: - grandchild = self._Item([]) - child = self._Item(grandchild) - item = self._Item([child]) - actual = walk(item, "child") - expected = [child, grandchild] - assert expected == list(actual) + with pytest.warns(BettyDeprecationWarning): + grandchild = self._Item([]) + child = self._Item(grandchild) + item = self._Item([child]) + actual = walk(item, "child") + expected = [child, grandchild] + assert expected == list(actual) class TestSliceToRange: diff --git a/betty/tests/test_jinja2.py b/betty/tests/test_jinja2.py index a17de5acf..0b481d546 100644 --- a/betty/tests/test_jinja2.py +++ b/betty/tests/test_jinja2.py @@ -24,6 +24,7 @@ Citation, ) from betty.tests import TemplateTestCase +from betty.warnings import BettyDeprecationWarning class TestJinja2Provider: @@ -97,13 +98,14 @@ def __str__(self) -> str: ], ) async def test(self, expected: str, template: str, data: WalkData) -> None: - async with self._render( - template_string=template, - data={ - "data": data, - }, - ) as (actual, _): - assert expected == actual + with pytest.warns(BettyDeprecationWarning): + async with self._render( + template_string=template, + data={ + "data": data, + }, + ) as (actual, _): + assert expected == actual class TestFilterParagraphs(TemplateTestCase): diff --git a/documentation/usage/templating/filters.rst b/documentation/usage/templating/filters.rst index de0e3d12e..c04923df7 100644 --- a/documentation/usage/templating/filters.rst +++ b/documentation/usage/templating/filters.rst @@ -31,4 +31,3 @@ In addition to Jinja2's built-in filters, Betty provides the following: - :py:func:`upper_camel_case_to_lower_camel_case ` - :py:func:`url ` - :py:func:`void_none ` -- :py:func:`walk `