From 004216696604df2c3350c5d1f885963313f17f78 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:06:21 +0200 Subject: [PATCH 01/37] feat(core): add TiLiA's rules for creating each kind the importers create point(), segment(), range_() and page() port TiLiA's validate_creation for point-like and segment-like components, the range timeline's _validate_component_creation, and the PDF importer's page check, with TiLiA's reasons word for word. point() takes the component's kind, which names it in the reason ("PDF page marker"), and formats times as TiLiA's format_media_time does. Part of TimeLineAnnotator/project-management#265 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/importers/_rules.py | 116 ++++++++++++ .../tests/importers/test_importers_rules.py | 172 ++++++++++++++++++ 2 files changed, 288 insertions(+) create mode 100644 packages/tilia-core/src/tilia_core/importers/_rules.py create mode 100644 packages/tilia-core/tests/importers/test_importers_rules.py diff --git a/packages/tilia-core/src/tilia_core/importers/_rules.py b/packages/tilia-core/src/tilia_core/importers/_rules.py new file mode 100644 index 000000000..a8175126e --- /dev/null +++ b/packages/tilia-core/src/tilia_core/importers/_rules.py @@ -0,0 +1,116 @@ +"""TiLiA's rules for creating a component of each kind the importers create. + +Each rule gives `(True, "")`, or `(False, reason)` with TiLiA's reason, word for +word, so that the core and TiLiA refuse the same rows in the same words. The +rules are those of `validate_creation` in TiLiA's point-like and segment-like +components, and of `_validate_component_creation` in its timelines. +""" + +from __future__ import annotations + +from collections.abc import Collection, Hashable + +Rule = tuple[bool, str] + +_OK: Rule = (True, "") + +# The names TiLiA's reasons give each point kind (its components' `frontend_name`). +_POINT_NAMES = { + "marker": "marker", + "harmony": "harmony", + "mode": "mode", + "beat": "beat", + "pdf_marker": "PDF page marker", +} + + +def point( + time: float, media_length: float, existing_times: Collection[float], *, kind: str +) -> Rule: + """A marker, harmony, mode, beat or PDF marker (`kind`) at `time`. + + The time must be from 0 to the media length, and no other component of the + same kind may have it: `existing_times` are theirs. + """ + if time > media_length: + return ( + False, + f"Time '{_media_time(time)}' is bigger than media time" + f" '{_media_time(media_length)}'", + ) + if time < 0: + return False, f"Time can't be negative. Got '{_media_time(time)}'" + if time in existing_times: + return ( + False, + f"There is already a {_POINT_NAMES[kind]} component at the selected position.", + ) + return _OK + + +def segment( + start: float, + end: float, + position: Hashable, + existing_positions: Collection[Hashable], + media_length: float, +) -> Rule: + """A hierarchy from `start` to `end`, at `position`: its `(start, end, level)`. + + The start and the end must not be beyond the media length, the end must be + after the start, and no other hierarchy may have the same position. + """ + start, end, media_length = float(start), float(end), float(media_length) + if start > media_length: + return False, f"Start time '{start}' is bigger than media time '{media_length}'" + if end > media_length: + return False, f"End time '{end}' is bigger than media time '{media_length}'" + if end <= start: + return False, f"End time '{end}' should be bigger than start time '{start}'" + if position in existing_positions: + return ( + False, + "There is already a hierarchy component at the selected position.", + ) + return _OK + + +def range_( + start: float, + end: float, + row_id: str | None, + row_ids: Collection[str], + pre_start: float | None, + post_end: float | None, + media_length: float, +) -> Rule: + """A range from `start` to `end` in the row `row_id`, one of `row_ids`. + + `pre_start` and `post_end` are checked when given. + """ + if not row_id or row_id not in row_ids: + return False, "Invalid row ID." + if start >= end: + return False, "Start time must be before end time." + if start < 0 or end > media_length: + return False, "Range is outside media bounds." + if pre_start is not None and pre_start < 0: + return False, "pre_start is outside media bounds." + if post_end is not None and post_end > media_length: + return False, "post_end is outside media bounds." + return _OK + + +def page(page_number: int, page_total: int) -> Rule: + """A PDF marker's page: one the PDF has, from 1 to `page_total`.""" + if not 1 <= page_number <= page_total: + return False, "The PDF has no page with that number." + return _OK + + +def _media_time(seconds: float) -> str: + """A time as TiLiA's `format_media_time` writes it: "01:05.0", "1:02:05.0".""" + seconds_and_fraction = f"{seconds % 60:.1f}".zfill(4) + minutes = int(float(seconds) // 60) + hours = f"{minutes // 60}:" if minutes >= 60 else "" + return f"{hours}{str(minutes % 60).zfill(2)}:{seconds_and_fraction}" diff --git a/packages/tilia-core/tests/importers/test_importers_rules.py b/packages/tilia-core/tests/importers/test_importers_rules.py new file mode 100644 index 000000000..3000a089a --- /dev/null +++ b/packages/tilia-core/tests/importers/test_importers_rules.py @@ -0,0 +1,172 @@ +import pytest + +from tilia_core.importers import _rules + +OK = (True, "") +MEDIA = 60.0 + + +class TestPoint: + @pytest.mark.parametrize("time", [0.0, 30.0, MEDIA]) + def test_a_time_from_0_to_the_media_length(self, time): + assert _rules.point(time, MEDIA, set(), kind="marker") == OK + + def test_a_time_beyond_the_media_length(self): + assert _rules.point(60.5, MEDIA, set(), kind="marker") == ( + False, + "Time '01:00.5' is bigger than media time '01:00.0'", + ) + + def test_times_of_an_hour_or_more_show_hours(self): + assert _rules.point(3725.0, 3600.0, set(), kind="marker") == ( + False, + "Time '1:02:05.0' is bigger than media time '1:00:00.0'", + ) + + def test_a_negative_time(self): + # TiLiA's format_media_time, as it is, shows -1 second as 59:59.0. + assert _rules.point(-1.0, MEDIA, set(), kind="marker") == ( + False, + "Time can't be negative. Got '59:59.0'", + ) + + @pytest.mark.parametrize( + "kind, name", + [ + ("marker", "marker"), + ("harmony", "harmony"), + ("mode", "mode"), + ("beat", "beat"), + ("pdf_marker", "PDF page marker"), + ], + ) + def test_a_time_another_component_has(self, kind, name): + assert _rules.point(1.0, MEDIA, {0.0, 1.0}, kind=kind) == ( + False, + f"There is already a {name} component at the selected position.", + ) + + @pytest.mark.parametrize("time", [0.0, MEDIA]) + def test_a_time_another_component_has_at_the_edges(self, time): + assert _rules.point(time, MEDIA, {time}, kind="marker")[0] is False + + def test_bounds_are_checked_before_other_components(self): + assert _rules.point(61.0, MEDIA, {61.0}, kind="marker") == ( + False, + "Time '01:01.0' is bigger than media time '01:00.0'", + ) + + def test_media_length_given_as_an_integer(self): + assert _rules.point(61.0, 60, set(), kind="marker") == ( + False, + "Time '01:01.0' is bigger than media time '01:00.0'", + ) + + +class TestSegment: + @pytest.mark.parametrize("start, end", [(0.0, 1.0), (1.0, MEDIA), (0.0, MEDIA)]) + def test_within_the_media_length(self, start, end): + assert _rules.segment(start, end, (start, end, 1), set(), MEDIA) == OK + + def test_a_start_beyond_the_media_length(self): + assert _rules.segment(61.0, 62.0, (61.0, 62.0, 1), set(), MEDIA) == ( + False, + "Start time '61.0' is bigger than media time '60.0'", + ) + + def test_an_end_beyond_the_media_length(self): + assert _rules.segment(1.0, 61.0, (1.0, 61.0, 1), set(), MEDIA) == ( + False, + "End time '61.0' is bigger than media time '60.0'", + ) + + @pytest.mark.parametrize("end", [2.0, 1.0]) + def test_an_end_not_after_its_start(self, end): + assert _rules.segment(2.0, end, (2.0, end, 1), set(), MEDIA) == ( + False, + f"End time '{end}' should be bigger than start time '2.0'", + ) + + def test_the_position_of_another_component(self): + existing = {(0.0, 1.0, 1), (1.0, 2.0, 1)} + + assert _rules.segment(1.0, 2.0, (1.0, 2.0, 1), existing, MEDIA) == ( + False, + "There is already a hierarchy component at the selected position.", + ) + + def test_the_same_times_at_another_level(self): + assert _rules.segment(1.0, 2.0, (1.0, 2.0, 2), {(1.0, 2.0, 1)}, MEDIA) == OK + + def test_times_are_checked_before_other_components(self): + position = (2.0, 1.0, 1) + + assert _rules.segment(2.0, 1.0, position, {position}, MEDIA) == ( + False, + "End time '1.0' should be bigger than start time '2.0'", + ) + + def test_numbers_given_as_integers_read_as_tilias_floats(self): + assert _rules.segment(61, 62, (61, 62, 1), set(), 60) == ( + False, + "Start time '61.0' is bigger than media time '60.0'", + ) + + +class TestRange: + ROWS = ["Ab12Cd", "Ef34Gh"] + + def check(self, start=1.0, end=2.0, row_id="Ab12Cd", pre_start=None, post_end=None): + return _rules.range_(start, end, row_id, self.ROWS, pre_start, post_end, MEDIA) + + def test_a_range_in_a_row(self): + assert self.check() == OK + + @pytest.mark.parametrize( + "start, end, pre_start, post_end", + [(0.0, MEDIA, None, None), (1.0, 2.0, 0.0, MEDIA), (0.0, MEDIA, 0.0, MEDIA)], + ) + def test_the_edges(self, start, end, pre_start, post_end): + assert self.check(start, end, pre_start=pre_start, post_end=post_end) == OK + + @pytest.mark.parametrize("row_id", ["", None, "Zz99Zz"]) + def test_a_row_the_timeline_doesnt_have(self, row_id): + assert self.check(row_id=row_id) == (False, "Invalid row ID.") + + @pytest.mark.parametrize("end", [1.0, 0.5]) + def test_an_end_not_after_its_start(self, end): + assert self.check(1.0, end) == (False, "Start time must be before end time.") + + @pytest.mark.parametrize("start, end", [(-0.5, 2.0), (1.0, 60.5)]) + def test_outside_the_media(self, start, end): + assert self.check(start, end) == (False, "Range is outside media bounds.") + + def test_a_pre_start_below_0(self): + assert self.check(pre_start=-0.5) == ( + False, + "pre_start is outside media bounds.", + ) + + def test_a_post_end_beyond_the_media_length(self): + assert self.check(post_end=60.5) == ( + False, + "post_end is outside media bounds.", + ) + + def test_the_row_is_checked_first(self): + assert self.check(2.0, 1.0, row_id="Zz99Zz") == (False, "Invalid row ID.") + + +class TestPage: + @pytest.mark.parametrize("page_number", [1, 2, 3]) + def test_a_page_the_pdf_has(self, page_number): + assert _rules.page(page_number, 3) == OK + + @pytest.mark.parametrize( + "page_number, page_total", [(0, 3), (4, 3), (-1, 3), (1, 0)] + ) + def test_a_page_the_pdf_doesnt_have(self, page_number, page_total): + assert _rules.page(page_number, page_total) == ( + False, + "The PDF has no page with that number.", + ) From ede5914dd43dcfee0f63ced764ac09dedfbb3958 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Tue, 13 Oct 2026 16:11:09 +0200 Subject: [PATCH 02/37] refactor(core): move the component rules to tilia_core.rules The rules are about to grow into the one set that everything creating or changing a component goes through: the importers, and later bulk edits and the reader and writer. So they leave the importers package. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- .../{importers/_rules.py => rules.py} | 0 .../test_importers_rules.py => test_rules.py} | 40 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) rename packages/tilia-core/src/tilia_core/{importers/_rules.py => rules.py} (100%) rename packages/tilia-core/tests/{importers/test_importers_rules.py => test_rules.py} (77%) diff --git a/packages/tilia-core/src/tilia_core/importers/_rules.py b/packages/tilia-core/src/tilia_core/rules.py similarity index 100% rename from packages/tilia-core/src/tilia_core/importers/_rules.py rename to packages/tilia-core/src/tilia_core/rules.py diff --git a/packages/tilia-core/tests/importers/test_importers_rules.py b/packages/tilia-core/tests/test_rules.py similarity index 77% rename from packages/tilia-core/tests/importers/test_importers_rules.py rename to packages/tilia-core/tests/test_rules.py index 3000a089a..f55d2f9f4 100644 --- a/packages/tilia-core/tests/importers/test_importers_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -1,6 +1,6 @@ import pytest -from tilia_core.importers import _rules +from tilia_core import rules OK = (True, "") MEDIA = 60.0 @@ -9,23 +9,23 @@ class TestPoint: @pytest.mark.parametrize("time", [0.0, 30.0, MEDIA]) def test_a_time_from_0_to_the_media_length(self, time): - assert _rules.point(time, MEDIA, set(), kind="marker") == OK + assert rules.point(time, MEDIA, set(), kind="marker") == OK def test_a_time_beyond_the_media_length(self): - assert _rules.point(60.5, MEDIA, set(), kind="marker") == ( + assert rules.point(60.5, MEDIA, set(), kind="marker") == ( False, "Time '01:00.5' is bigger than media time '01:00.0'", ) def test_times_of_an_hour_or_more_show_hours(self): - assert _rules.point(3725.0, 3600.0, set(), kind="marker") == ( + assert rules.point(3725.0, 3600.0, set(), kind="marker") == ( False, "Time '1:02:05.0' is bigger than media time '1:00:00.0'", ) def test_a_negative_time(self): # TiLiA's format_media_time, as it is, shows -1 second as 59:59.0. - assert _rules.point(-1.0, MEDIA, set(), kind="marker") == ( + assert rules.point(-1.0, MEDIA, set(), kind="marker") == ( False, "Time can't be negative. Got '59:59.0'", ) @@ -41,23 +41,23 @@ def test_a_negative_time(self): ], ) def test_a_time_another_component_has(self, kind, name): - assert _rules.point(1.0, MEDIA, {0.0, 1.0}, kind=kind) == ( + assert rules.point(1.0, MEDIA, {0.0, 1.0}, kind=kind) == ( False, f"There is already a {name} component at the selected position.", ) @pytest.mark.parametrize("time", [0.0, MEDIA]) def test_a_time_another_component_has_at_the_edges(self, time): - assert _rules.point(time, MEDIA, {time}, kind="marker")[0] is False + assert rules.point(time, MEDIA, {time}, kind="marker")[0] is False def test_bounds_are_checked_before_other_components(self): - assert _rules.point(61.0, MEDIA, {61.0}, kind="marker") == ( + assert rules.point(61.0, MEDIA, {61.0}, kind="marker") == ( False, "Time '01:01.0' is bigger than media time '01:00.0'", ) def test_media_length_given_as_an_integer(self): - assert _rules.point(61.0, 60, set(), kind="marker") == ( + assert rules.point(61.0, 60, set(), kind="marker") == ( False, "Time '01:01.0' is bigger than media time '01:00.0'", ) @@ -66,23 +66,23 @@ def test_media_length_given_as_an_integer(self): class TestSegment: @pytest.mark.parametrize("start, end", [(0.0, 1.0), (1.0, MEDIA), (0.0, MEDIA)]) def test_within_the_media_length(self, start, end): - assert _rules.segment(start, end, (start, end, 1), set(), MEDIA) == OK + assert rules.segment(start, end, (start, end, 1), set(), MEDIA) == OK def test_a_start_beyond_the_media_length(self): - assert _rules.segment(61.0, 62.0, (61.0, 62.0, 1), set(), MEDIA) == ( + assert rules.segment(61.0, 62.0, (61.0, 62.0, 1), set(), MEDIA) == ( False, "Start time '61.0' is bigger than media time '60.0'", ) def test_an_end_beyond_the_media_length(self): - assert _rules.segment(1.0, 61.0, (1.0, 61.0, 1), set(), MEDIA) == ( + assert rules.segment(1.0, 61.0, (1.0, 61.0, 1), set(), MEDIA) == ( False, "End time '61.0' is bigger than media time '60.0'", ) @pytest.mark.parametrize("end", [2.0, 1.0]) def test_an_end_not_after_its_start(self, end): - assert _rules.segment(2.0, end, (2.0, end, 1), set(), MEDIA) == ( + assert rules.segment(2.0, end, (2.0, end, 1), set(), MEDIA) == ( False, f"End time '{end}' should be bigger than start time '2.0'", ) @@ -90,24 +90,24 @@ def test_an_end_not_after_its_start(self, end): def test_the_position_of_another_component(self): existing = {(0.0, 1.0, 1), (1.0, 2.0, 1)} - assert _rules.segment(1.0, 2.0, (1.0, 2.0, 1), existing, MEDIA) == ( + assert rules.segment(1.0, 2.0, (1.0, 2.0, 1), existing, MEDIA) == ( False, "There is already a hierarchy component at the selected position.", ) def test_the_same_times_at_another_level(self): - assert _rules.segment(1.0, 2.0, (1.0, 2.0, 2), {(1.0, 2.0, 1)}, MEDIA) == OK + assert rules.segment(1.0, 2.0, (1.0, 2.0, 2), {(1.0, 2.0, 1)}, MEDIA) == OK def test_times_are_checked_before_other_components(self): position = (2.0, 1.0, 1) - assert _rules.segment(2.0, 1.0, position, {position}, MEDIA) == ( + assert rules.segment(2.0, 1.0, position, {position}, MEDIA) == ( False, "End time '1.0' should be bigger than start time '2.0'", ) def test_numbers_given_as_integers_read_as_tilias_floats(self): - assert _rules.segment(61, 62, (61, 62, 1), set(), 60) == ( + assert rules.segment(61, 62, (61, 62, 1), set(), 60) == ( False, "Start time '61.0' is bigger than media time '60.0'", ) @@ -117,7 +117,7 @@ class TestRange: ROWS = ["Ab12Cd", "Ef34Gh"] def check(self, start=1.0, end=2.0, row_id="Ab12Cd", pre_start=None, post_end=None): - return _rules.range_(start, end, row_id, self.ROWS, pre_start, post_end, MEDIA) + return rules.range_(start, end, row_id, self.ROWS, pre_start, post_end, MEDIA) def test_a_range_in_a_row(self): assert self.check() == OK @@ -160,13 +160,13 @@ def test_the_row_is_checked_first(self): class TestPage: @pytest.mark.parametrize("page_number", [1, 2, 3]) def test_a_page_the_pdf_has(self, page_number): - assert _rules.page(page_number, 3) == OK + assert rules.page(page_number, 3) == OK @pytest.mark.parametrize( "page_number, page_total", [(0, 3), (4, 3), (-1, 3), (1, 0)] ) def test_a_page_the_pdf_doesnt_have(self, page_number, page_total): - assert _rules.page(page_number, page_total) == ( + assert rules.page(page_number, page_total) == ( False, "The PDF has no page with that number.", ) From 34fc21264517ad106907b26bb1d9f6339f97e09b Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Tue, 13 Oct 2026 16:46:46 +0200 Subject: [PATCH 03/37] feat(core): one set of rules for the components the importers create tilia_core.rules now holds every rule for a valid marker, hierarchy, range, harmony, mode, PDF marker and beat: one function per kind, which takes the media length, the timeline's other components and the attributes to check, and gives (True, "") or (False, reason) for the first rule broken. Where TiLiA's checks at creation, in set_data and in its operations disagree, the strictest applies, so some rows TiLiA's importers accept are refused: - a time of nan, with the text the kind's importer gives a time that isn't a number (not_valid, which the importers can use for that text too); - a label, comments or custom text that isn't text, and a color Qt can't read, checked without Qt against the forms and names QColor accepts; - a hierarchy that starts before 0, has a level below 1, a pre_start outside 0 to its start or a post_end outside its end to the media length, or crosses another (TiLiA's get_boundary_conflicts); - a range whose pre_start is after its start or whose post_end is before its end, or whose joined_right isn't the next range in its row; - harmony and mode values TiLiA refuses in set_data: steps, accidentals, levels, display modes, font types, and qualities, inversions and mode types read from the installed music21, imported only when a harmony or a mode is checked. Without music21, the other rules are checked, and CantCheck names the command that installs the harmony extra. A negative time is now shown with a minus sign ("-00:01.0") rather than wrapped around an hour. A broken rule between components carries the ids of the others involved (Outcome.others). range_rows checks a range timeline's rows. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 649 ++++++++++++-- packages/tilia-core/tests/test_rules.py | 917 ++++++++++++++++++-- 2 files changed, 1434 insertions(+), 132 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index a8175126e..0f3cbfcef 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -1,20 +1,53 @@ -"""TiLiA's rules for creating a component of each kind the importers create. +"""The one set of rules for a valid component. -Each rule gives `(True, "")`, or `(False, reason)` with TiLiA's reason, word for -word, so that the core and TiLiA refuse the same rows in the same words. The -rules are those of `validate_creation` in TiLiA's point-like and segment-like -components, and of `_validate_component_creation` in its timelines. +Everything that creates or changes a component goes through these rules and +keeps none of its own: the importers, the `tilia` command, bulk edits, and the +reader and the writer of `.tla` files. TiLiA checks a component in several +places (when it creates one, when it sets a value, in its editing operations), +which don't always agree; the rules are the strictest of them and of the +format's schema. + +There is one function per kind the importers create. Each takes the media +length, the timeline's other components (not the one checked) and the +attributes of the component to check, and gives the first rule they break: +`(False, reason)`, or `(True, "")` when they break none. The reasons are +TiLiA's, word for word, where TiLiA has one. + +A harmony's quality and inversion and a mode's type are checked against the +installed music21, as TiLiA checks them, so checking a harmony or a mode needs +the `harmony` extra. Without it, the other rules are checked, and when none is +broken, `CantCheck` is raised. music21 is imported only when a harmony or a mode +is checked. """ from __future__ import annotations -from collections.abc import Collection, Hashable +import importlib.util +import math +import string +from collections.abc import Collection, Iterable, Mapping, Sequence +from types import ModuleType +from typing import TYPE_CHECKING, Any -Rule = tuple[bool, str] +if TYPE_CHECKING: + from tilia_core import tla -_OK: Rule = (True, "") +INSTALL_HARMONY = 'pip install "tilia-core[harmony]"' -# The names TiLiA's reasons give each point kind (its components' `frontend_name`). +# TiLiA's values (tilia/timelines/harmony/constants.py). Qualities and mode types +# are music21's. +HARMONY_ACCIDENTALS = (2, 1, 0, -1, -2) +HARMONY_DISPLAY_MODES = ("letter", "roman", "custom") +FONT_TYPES = ("analytic", "normal") +_STEPS = range(7) +_LEVELS = (1, 2) +_NOT_MODE_TYPES = ("ionian", "aeolian") +# Qualities TiLiA keeps in root position, whatever their intervals. +_NO_INVERSION_QUALITIES = frozenset( + {"Italian", "French", "German", "Neapolitan", "Tristan", "power"} +) + +# The names TiLiA's reasons give the point kinds (their `frontend_name`). _POINT_NAMES = { "marker": "marker", "harmony": "harmony", @@ -23,94 +56,580 @@ "pdf_marker": "PDF page marker", } +_TEXTS = ("label", "comments", "custom_text") +_SEGMENT_TIMES = ("start", "end", "pre_start", "post_end") +_UNSET_TIMES = ("pre_start", "post_end") # None: they follow the start and the end + +# How each kind's importer words a value it can't read (tilia/parsers/csv/), +# leaving out what names the row. +_NOT_VALID = { + "marker": "{value} is not a valid {attribute}", + "hierarchy": "'{value}' is not a valid {attribute}", + "range": "{value} is not a valid {attribute}", + "harmony": "{value} is not a valid {attribute}.", + "mode": "{value} is not a valid {attribute}.", + "pdf_marker": "{value} is not a valid {attribute}.", + "beat": "'{value}' is not a valid {attribute}", +} + +_ROW_ID_LENGTH = 6 +_ROW_ID_CHARACTERS = frozenset(string.ascii_letters + string.digits) + +# The colors Qt reads: #RGB, #RRGGBB, #AARRGGBB, #RRRGGGBBB and #RRRRGGGGBBBB, +# and the SVG color names with "transparent" (`QColor.colorNames()`). +_HEX_LENGTHS = (3, 6, 8, 9, 12) +_HEX_DIGITS = frozenset(string.hexdigits) +_COLOR_NAMES = frozenset( + { + "aliceblue", "antiquewhite", "aqua", "aquamarine", "azure", "beige", "bisque", + "black", "blanchedalmond", "blue", "blueviolet", "brown", "burlywood", + "cadetblue", "chartreuse", "chocolate", "coral", "cornflowerblue", "cornsilk", + "crimson", "cyan", "darkblue", "darkcyan", "darkgoldenrod", "darkgray", + "darkgreen", "darkgrey", "darkkhaki", "darkmagenta", "darkolivegreen", + "darkorange", "darkorchid", "darkred", "darksalmon", "darkseagreen", + "darkslateblue", "darkslategray", "darkslategrey", "darkturquoise", + "darkviolet", "deeppink", "deepskyblue", "dimgray", "dimgrey", "dodgerblue", + "firebrick", "floralwhite", "forestgreen", "fuchsia", "gainsboro", "ghostwhite", + "gold", "goldenrod", "gray", "green", "greenyellow", "grey", "honeydew", + "hotpink", "indianred", "indigo", "ivory", "khaki", "lavender", "lavenderblush", + "lawngreen", "lemonchiffon", "lightblue", "lightcoral", "lightcyan", + "lightgoldenrodyellow", "lightgray", "lightgreen", "lightgrey", "lightpink", + "lightsalmon", "lightseagreen", "lightskyblue", "lightslategray", + "lightslategrey", "lightsteelblue", "lightyellow", "lime", "limegreen", "linen", + "magenta", "maroon", "mediumaquamarine", "mediumblue", "mediumorchid", + "mediumpurple", "mediumseagreen", "mediumslateblue", "mediumspringgreen", + "mediumturquoise", "mediumvioletred", "midnightblue", "mintcream", "mistyrose", + "moccasin", "navajowhite", "navy", "oldlace", "olive", "olivedrab", "orange", + "orangered", "orchid", "palegoldenrod", "palegreen", "paleturquoise", + "palevioletred", "papayawhip", "peachpuff", "peru", "pink", "plum", + "powderblue", "purple", "red", "rosybrown", "royalblue", "saddlebrown", + "salmon", "sandybrown", "seagreen", "seashell", "sienna", "silver", "skyblue", + "slateblue", "slategray", "slategrey", "snow", "springgreen", "steelblue", + "tan", "teal", "thistle", "tomato", "transparent", "turquoise", "violet", + "wheat", "white", "whitesmoke", "yellow", "yellowgreen", + } +) # fmt: skip + + +class Outcome(tuple[bool, str]): + """What checking the rules gives: `(True, "")`, or `(False, reason)`. + + `others` are the ids of the other components a broken rule is about: those + at the same time or place, the hierarchies it crosses, the ranges it should + join. + """ + + others: tuple[str, ...] + + def __new__(cls, ok: bool, reason: str = "", others: Iterable[str] = ()) -> Outcome: + outcome = super().__new__(cls, (ok, reason)) + outcome.others = tuple(others) + return outcome + + @property + def ok(self) -> bool: + return self[0] + + @property + def reason(self) -> str: + return self[1] + + +OK = Outcome(True) + + +class CantCheck(Exception): + """The rules that need music21 can't be checked: the `harmony` extra is missing. + + This isn't a broken rule: the component may well keep to them. + """ + + command = INSTALL_HARMONY + + def __init__(self, kind: str) -> None: + super().__init__(f"checking a {kind} needs: {INSTALL_HARMONY}") + self.kind = kind + + +# The private functions give the first rule they find broken, or None, so that +# `or` checks them in order. + + +def marker( + media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] +) -> Outcome: + """A marker.""" + return _point("marker", media_length, others, attrs) or OK -def point( - time: float, media_length: float, existing_times: Collection[float], *, kind: str -) -> Rule: - """A marker, harmony, mode, beat or PDF marker (`kind`) at `time`. - The time must be from 0 to the media length, and no other component of the - same kind may have it: `existing_times` are theirs. +def beat( + media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] +) -> Outcome: + """A beat. Its measure mark and beat unit follow the measure table's rules.""" + return _point("beat", media_length, others, attrs) or OK + + +def pdf_marker( + media_length: float, + others: Iterable[tla.Component], + attrs: Mapping[str, Any], + *, + page_count: int, +) -> Outcome: + """A PDF marker, in a PDF of `page_count` pages: 0 for a PDF that can't be read.""" + return ( + _point("pdf_marker", media_length, others, attrs) + or _page(attrs.get("page_number"), page_count) + or OK + ) + + +def harmony( + media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] +) -> Outcome: + """A harmony. Without music21, raises `CantCheck` when no other rule is broken.""" + music21 = _music21() + broken = ( + _point("harmony", media_length, others, attrs) + or _one_of("harmony", attrs, "step", _STEPS) + or _one_of("harmony", attrs, "applied_to", _STEPS) + or _one_of("harmony", attrs, "accidental", HARMONY_ACCIDENTALS) + or (_quality_and_inversion(music21, attrs) if music21 else None) + or _one_of("harmony", attrs, "level", _LEVELS) + or _one_of("harmony", attrs, "display_mode", HARMONY_DISPLAY_MODES) + or _one_of("harmony", attrs, "custom_text_font_type", FONT_TYPES) + ) + return _checked("harmony", broken, music21) + + +def mode( + media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] +) -> Outcome: + """A key. Without music21, raises `CantCheck` when no other rule is broken.""" + music21 = _music21() + broken = ( + _point("mode", media_length, others, attrs) + or _one_of("mode", attrs, "step", _STEPS) + or _one_of("mode", attrs, "accidental", HARMONY_ACCIDENTALS) + or (_mode_type(music21, attrs) if music21 else None) + or _one_of("mode", attrs, "level", _LEVELS) + ) + return _checked("mode", broken, music21) + + +def hierarchy( + media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] +) -> Outcome: + """A hierarchy. Hierarchies nest: one that overlaps another lies inside it, + at a lower level.""" + broken = _every_kind("hierarchy", attrs, _SEGMENT_TIMES) + if broken: + return broken + start, end, level = attrs["start"], attrs["end"], attrs.get("level") + hierarchies = [other for other in others if other.kind == "hierarchy"] + return ( + _hierarchy_times(start, end, media_length) + or _hierarchy_level(level) + or _pre_start(attrs.get("pre_start"), start) + or _post_end(attrs.get("post_end"), end, media_length) + or _same_place(start, end, level, hierarchies) + or _crossed(start, end, level, hierarchies) + or OK + ) + + +def range_( + media_length: float, + others: Iterable[tla.Component], + attrs: Mapping[str, Any], + *, + rows: Sequence[Mapping[str, Any]], +) -> Outcome: + """A range, in a timeline whose attribute `rows` is `rows`. + + Ranges in a row may overlap. """ + broken = _every_kind("range", attrs, _SEGMENT_TIMES) + if broken: + return broken + start, end = attrs["start"], attrs["end"] + row_ids = [row.get("id") for row in rows] + return ( + _range_place(attrs.get("row_id"), row_ids, start, end, media_length) + or _pre_start(attrs.get("pre_start"), start) + or _post_end(attrs.get("post_end"), end, media_length) + or _joined(attrs, others, rows) + or OK + ) + + +def range_rows(rows: Sequence[Mapping[str, Any]]) -> Outcome: + """A range timeline's rows: each with its own id of six letters and digits, + a name, and a color or none.""" + ids: set[str] = set() + for row in rows: + row_id = row.get("id") + if not _is_row_id(row_id): + return _broken(f"{row_id} is not a valid row id") + if row_id in ids: + return _broken(f"{row_id} is the id of two rows") + ids.add(row_id) + name = row.get("name") + if not isinstance(name, str): + return _broken(f"{name} is not a valid row name") + if not color(row.get("color")).ok: + # TiLiA's text for a row's color, which isn't the components' one. + return _broken(f"'{row.get('color')}' is not a valid color.") + return OK + + +def color(value: Any) -> Outcome: + """A color: the default (None or ""), or one Qt reads, checked without Qt. + + An importer keeps a row whose color isn't one, with the kind's default color + instead, and a note. + """ + if value is None or value == "": + return OK + if isinstance(value, str) and _qt_reads(value): + return OK + return _broken(f"{value} is not a valid color.") + + +def not_valid(kind: str, attribute: str, value: Any) -> str: + """How the importer of `kind` words a value of `attribute` it can't read. + + The rules give the same text for a value that isn't one, such as a time of + nan. An importer gives it for a value it can't parse, so the two read alike. + """ + if kind == "range" and attribute in _SEGMENT_TIMES: + attribute = "time" # as the range importer calls them + return _NOT_VALID[kind].format(value=value, attribute=attribute.replace("_", " ")) + + +def _point( + kind: str, + media_length: float, + others: Iterable[tla.Component], + attrs: Mapping[str, Any], +) -> Outcome | None: + """The rules for every kind, the time's bounds, and no other at the same time.""" + broken = _every_kind(kind, attrs, ("time",)) + if broken: + return broken + time = attrs["time"] + if time < 0: + return _broken(f"Time can't be negative. Got '{_media_time(time)}'") if time > media_length: - return ( - False, + return _broken( f"Time '{_media_time(time)}' is bigger than media time" - f" '{_media_time(media_length)}'", + f" '{_media_time(media_length)}'" ) - if time < 0: - return False, f"Time can't be negative. Got '{_media_time(time)}'" - if time in existing_times: - return ( + same = [ + other.id + for other in others + if other.kind == kind and other.attrs.get("time") == time + ] + if same: + return Outcome( False, f"There is already a {_POINT_NAMES[kind]} component at the selected position.", + same, ) - return _OK + return None -def segment( - start: float, - end: float, - position: Hashable, - existing_positions: Collection[Hashable], - media_length: float, -) -> Rule: - """A hierarchy from `start` to `end`, at `position`: its `(start, end, level)`. +def _every_kind( + kind: str, attrs: Mapping[str, Any], times: Iterable[str] +) -> Outcome | None: + """Times are finite numbers, texts are text, and a color is one.""" + for name in times: + value = attrs.get(name) + if value is None and name in _UNSET_TIMES: + continue + if not _is_number(value): + return _broken(not_valid(kind, name, value)) + for name in _TEXTS: + if name in attrs and not isinstance(attrs[name], str): + return _broken(not_valid(kind, name, attrs[name])) + if "color" in attrs: + outcome = color(attrs["color"]) + if not outcome.ok: + return outcome + return None - The start and the end must not be beyond the media length, the end must be - after the start, and no other hierarchy may have the same position. - """ + +def _page(page_number: Any, page_count: int) -> Outcome | None: + if not _is_integer(page_number): + return _broken(not_valid("pdf_marker", "page_number", page_number)) + if not 1 <= page_number <= page_count: + return _broken("The PDF has no page with that number.") + return None + + +def _one_of( + kind: str, attrs: Mapping[str, Any], name: str, allowed: Collection[int | str] +) -> Outcome | None: + """`name`, when the component has it, is one of `allowed`, as it is: 1.0 isn't 1.""" + if name not in attrs: + return None + value = attrs[name] + if (_is_integer(value) or isinstance(value, str)) and value in allowed: + return None + return _broken(not_valid(kind, name, value)) + + +def _music21() -> ModuleType | None: + """music21, with what the rules read of it; None without the `harmony` extra.""" + if importlib.util.find_spec("music21") is None: + return None + import music21.harmony + import music21.key + + return music21 + + +def _checked(kind: str, broken: Outcome | None, music21: ModuleType | None) -> Outcome: + if broken: + return broken + if music21 is None: + raise CantCheck(kind) + return OK + + +def _quality_and_inversion( + music21: ModuleType, attrs: Mapping[str, Any] +) -> Outcome | None: + """The quality is one of music21's `CHORD_TYPES`, and the inversion one it has.""" + quality = attrs.get("quality", "major") + if not (isinstance(quality, str) and quality in music21.harmony.CHORD_TYPES): + return _broken(not_valid("harmony", "quality", quality)) + inversion = attrs.get("inversion", 0) + if not (_is_integer(inversion) and 0 <= inversion <= _inversions(music21, quality)): + return _broken(not_valid("harmony", "inversion", inversion)) + return None + + +def _inversions(music21: ModuleType, quality: str) -> int: + """A quality's inversions, as TiLiA's `get_inversion_amount` counts them.""" + if quality in _NO_INVERSION_QUALITIES: + return 0 + intervals = str(music21.harmony.CHORD_TYPES[quality][0]) + return len(intervals.split(",")) - 1 + + +def _mode_type(music21: ModuleType, attrs: Mapping[str, Any]) -> Outcome | None: + """The type is one of music21's `modeSharpsAlter`, but ionian and aeolian.""" + types = [ + name for name in music21.key.modeSharpsAlter if name not in _NOT_MODE_TYPES + ] + return _one_of("mode", attrs, "type", types) + + +def _hierarchy_times(start: float, end: float, media_length: float) -> Outcome | None: + # TiLiA shows these as floats, whatever they were given as. start, end, media_length = float(start), float(end), float(media_length) if start > media_length: - return False, f"Start time '{start}' is bigger than media time '{media_length}'" + return _broken( + f"Start time '{start}' is bigger than media time '{media_length}'" + ) if end > media_length: - return False, f"End time '{end}' is bigger than media time '{media_length}'" + return _broken(f"End time '{end}' is bigger than media time '{media_length}'") + if start < 0: + return _broken(f"Start time can't be negative. Got '{start}'") if end <= start: - return False, f"End time '{end}' should be bigger than start time '{start}'" - if position in existing_positions: - return ( + return _broken(f"End time '{end}' should be bigger than start time '{start}'") + return None + + +def _hierarchy_level(level: Any) -> Outcome | None: + if not _is_integer(level): + return _broken(not_valid("hierarchy", "level", level)) + if level < 1: + return _broken(f"Level can't be below 1. Got '{level}'") + return None + + +def _pre_start(pre_start: float | None, start: float) -> Outcome | None: + """`pre_start`, when set, is from 0 to the start.""" + if pre_start is None: + return None + if pre_start < 0: + return _broken("pre_start is outside media bounds.") + if pre_start > start: + return _broken("pre_start can't be after start time.") + return None + + +def _post_end( + post_end: float | None, end: float, media_length: float +) -> Outcome | None: + """`post_end`, when set, is from the end to the media length.""" + if post_end is None: + return None + if post_end > media_length: + return _broken("post_end is outside media bounds.") + if post_end < end: + return _broken("post_end can't be before end time.") + return None + + +def _same_place( + start: float, end: float, level: int, hierarchies: Iterable[tla.Component] +) -> Outcome | None: + same = [ + other.id + for other in hierarchies + if (other.attrs.get("start"), other.attrs.get("end"), other.attrs.get("level")) + == (start, end, level) + ] + if same: + return Outcome( False, "There is already a hierarchy component at the selected position.", + same, ) - return _OK + return None -def range_( - start: float, - end: float, - row_id: str | None, - row_ids: Collection[str], - pre_start: float | None, - post_end: float | None, - media_length: float, -) -> Rule: - """A range from `start` to `end` in the row `row_id`, one of `row_ids`. +def _crossed( + start: float, end: float, level: int, hierarchies: Iterable[tla.Component] +) -> Outcome | None: + crossed = [ + other.id for other in hierarchies if _cross(start, end, level, other.attrs) + ] + if crossed: + return Outcome( + False, "Component would overlap with existing component.", crossed + ) + return None + - `pre_start` and `post_end` are checked when given. +def _cross(start: float, end: float, level: int, other: Mapping[str, Any]) -> bool: + """Whether two hierarchies overlap, and neither lies inside the other, lower. + + This is when TiLiA's `get_boundary_conflicts` finds a conflict between them. """ + other_start, other_end = other.get("start"), other.get("end") + other_level = other.get("level") + if not ( + _is_number(other_start) and _is_number(other_end) and _is_integer(other_level) + ): + return False + if not (start < other_end and other_start < end): + return False + inside = other_start <= start and end <= other_end and level < other_level + around = start <= other_start and other_end <= end and other_level < level + return not (inside or around) + + +def _range_place( + row_id: Any, row_ids: Sequence[Any], start: float, end: float, media_length: float +) -> Outcome | None: if not row_id or row_id not in row_ids: - return False, "Invalid row ID." + return _broken("Invalid row ID.") if start >= end: - return False, "Start time must be before end time." + return _broken("Start time must be before end time.") if start < 0 or end > media_length: - return False, "Range is outside media bounds." - if pre_start is not None and pre_start < 0: - return False, "pre_start is outside media bounds." - if post_end is not None and post_end > media_length: - return False, "post_end is outside media bounds." - return _OK + return _broken("Range is outside media bounds.") + return None -def page(page_number: int, page_total: int) -> Rule: - """A PDF marker's page: one the PDF has, from 1 to `page_total`.""" - if not 1 <= page_number <= page_total: - return False, "The PDF has no page with that number." - return _OK +def _joined( + attrs: Mapping[str, Any], + others: Iterable[tla.Component], + rows: Sequence[Mapping[str, Any]], +) -> Outcome | None: + """`joined_right`, when set, is a next range in the row, starting at this one's end. + + The next ranges are those of the row that start first after this one starts. + The reasons are the range importer's (`_apply_pending_joins`). + """ + joined = attrs.get("joined_right") + if joined is None: + return None + row_id, start, end = attrs["row_id"], attrs["start"], attrs["end"] + row = next((row.get("name") for row in rows if row.get("id") == row_id), row_id) + later = [ + other + for other in others + if other.kind == "range" + and other.attrs.get("row_id") == row_id + and _is_number(other.attrs.get("start")) + and other.attrs["start"] > start + ] + if not later: + return _broken( + f"joined_with_next=true on last range of row {row!r};" + " no next neighbor to join with" + ) + next_start = min(other.attrs["start"] for other in later) + following = [other.id for other in later if other.attrs["start"] == next_start] + if next_start != end: + return Outcome( + False, + f"joined_with_next=true but next range on row {row!r} starts at" + f" {float(next_start)} (must equal {float(end)} for ranges to join)", + following, + ) + if joined not in following: + return Outcome( + False, + "joined_with_next=true but the range it joins isn't the next range" + f" on row {row!r}", + following, + ) + return None + + +def _qt_reads(text: str) -> bool: + """Whether Qt reads `text` as a color, as `QColor(text).isValid()` says.""" + if text.startswith("#"): + digits = text[1:] + return len(digits) in _HEX_LENGTHS and all(c in _HEX_DIGITS for c in digits) + # Qt reads a name of up to 255 UTF-16 units, in any case, leaving out spaces + # and tabs. A NUL, or a character beyond Latin-1, ends it. + if len(text.encode("utf-16-le")) // 2 > 255: + return False + name = [] + for character in text: + if character in " \t": + continue + if character == "\0" or ord(character) > 0xFF: + break + name.append(character.lower()) + return "".join(name) in _COLOR_NAMES def _media_time(seconds: float) -> str: - """A time as TiLiA's `format_media_time` writes it: "01:05.0", "1:02:05.0".""" + """A time as TiLiA's `format_media_time` shows it ("01:05.0", "1:02:05.0"), + with a minus sign before a negative one ("-00:01.0").""" + if seconds < 0: + return "-" + _media_time(-seconds) seconds_and_fraction = f"{seconds % 60:.1f}".zfill(4) minutes = int(float(seconds) // 60) hours = f"{minutes // 60}:" if minutes >= 60 else "" return f"{hours}{str(minutes % 60).zfill(2)}:{seconds_and_fraction}" + + +def _is_number(value: Any) -> bool: + """A finite number, as JSON stores one: not a bool, nan or an infinity.""" + if isinstance(value, bool): + return False + return isinstance(value, int) or (isinstance(value, float) and math.isfinite(value)) + + +def _is_integer(value: Any) -> bool: + return isinstance(value, int) and not isinstance(value, bool) + + +def _is_row_id(value: Any) -> bool: + return ( + isinstance(value, str) + and len(value) == _ROW_ID_LENGTH + and all(c in _ROW_ID_CHARACTERS for c in value) + ) + + +def _broken(reason: str) -> Outcome: + return Outcome(False, reason) diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index f55d2f9f4..bb66c9ab9 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -1,172 +1,955 @@ +import importlib.util +import itertools + import pytest -from tilia_core import rules +from tilia_core import rules, tla OK = (True, "") MEDIA = 60.0 +PAGES = 3 +ROW = "Ab12Cd" +ROWS = [{"id": ROW, "name": "Strings"}, {"id": "Ef34Gh", "name": "Winds"}] +INSTALL = 'pip install "tilia-core[harmony]"' + +POINT_KINDS = ["marker", "harmony", "mode", "pdf_marker", "beat"] +POINT_NAMES = { + "marker": "marker", + "harmony": "harmony", + "mode": "mode", + "pdf_marker": "PDF page marker", + "beat": "beat", +} +# Valid attributes for each kind, which a test changes one at a time. +VALID = { + "marker": {"time": 1.0}, + "harmony": {"time": 1.0}, + "mode": {"time": 1.0}, + "pdf_marker": {"time": 1.0, "page_number": 1}, + "beat": {"time": 1.0}, + "hierarchy": {"start": 1.0, "end": 2.0, "level": 1}, + "range": {"start": 1.0, "end": 2.0, "row_id": ROW}, +} + + +def check(kind, others=(), media_length=MEDIA, **attrs): + """Checks a component of `kind` with VALID's attributes, changed by `attrs`.""" + if kind in ("harmony", "mode"): + pytest.importorskip("music21", reason="needs the harmony extra") + attrs = {**VALID[kind], **attrs} + if kind == "pdf_marker": + return rules.pdf_marker(media_length, others, attrs, page_count=PAGES) + if kind == "range": + return rules.range_(media_length, others, attrs, rows=ROWS) + return getattr(rules, kind)(media_length, others, attrs) + + +def component(kind, **attrs): + return tla.Component(id=tla.new_id(), kind=kind, attrs=attrs) + + +def hierarchy(start, end, level, **attrs): + return component("hierarchy", start=start, end=end, level=level, **attrs) + + +def range_in(row_id, start, end, **attrs): + return component("range", start=start, end=end, row_id=row_id, **attrs) + + +@pytest.fixture +def music21(): + return pytest.importorskip("music21", reason="needs the harmony extra") + + +@pytest.fixture +def without_music21(monkeypatch): + find_spec = importlib.util.find_spec + + def find_spec_without_music21(name, *args, **kwargs): + if name.partition(".")[0] == "music21": + return None + return find_spec(name, *args, **kwargs) + + monkeypatch.setattr(importlib.util, "find_spec", find_spec_without_music21) + + +# TiLiA's creation rules, which its importers apply: `validate_creation` of its +# point-like and segment-like components, and the range timeline's +# `_validate_component_creation`. They say whether TiLiA creates a component, +# for the tests that show a rule the core adds; what they don't take, they +# don't check. + + +def tilia_creates_point(time, existing_times=(), media_length=MEDIA, **_): + if time > media_length: + return False + elif time < 0: + return False + return time not in existing_times + + +def tilia_creates_hierarchy(start, end, level, existing=(), media_length=MEDIA, **_): + if start > media_length: + return False + elif end > media_length: + return False + elif end <= start: + return False + return (start, end, level) not in existing + + +def tilia_creates_range( + start, end, row_id, pre_start=None, post_end=None, media_length=MEDIA, **_ +): + if not row_id or row_id not in [row["id"] for row in ROWS]: + return False + if start >= end: + return False + if start < 0 or end > media_length: + return False + if pre_start is not None and pre_start < 0: + return False + if post_end is not None and post_end > media_length: + return False + return True + + +def tilia_boundary_conflicts(hierarchies): + """`HierarchyTLComponentManager.get_boundary_conflicts`, on (start, end, level)s.""" + conflicts = [] + for hrc1, hrc2 in itertools.combinations(hierarchies, 2): + start1, end1, level1 = hrc1 + start2, end2, level2 = hrc2 + if start1 < start2 < end1 or start1 < end2 < end1: + if level2 >= level1: + conflicts.append((hrc1, hrc2)) + elif not start1 > start2 and end1 < end2: + conflicts.append((hrc1, hrc2)) + if start2 < start1 < end2 or start2 < end1 < end2: + if level1 >= level2: + conflicts.append((hrc2, hrc1)) + elif not start2 > start1 and end2 < end1: + conflicts.append((hrc2, hrc1)) + if start1 == start2 and end1 == end2 and level1 == level2: + conflicts.append((hrc1, hrc2)) + return conflicts + + +class TestOutcome: + def test_a_kept_rule_is_true_and_no_reason(self): + assert rules.OK == (True, "") + assert rules.OK.others == () + + def test_a_broken_rule_unpacks_as_its_reason(self): + ok, reason = rules.Outcome(False, "Invalid row ID.") + + assert (ok, reason) == (False, "Invalid row ID.") + + def test_a_broken_rule_names_the_other_components(self): + outcome = rules.Outcome(False, "Component would overlap", ["a", "b"]) + + assert outcome == (False, "Component would overlap") + assert outcome.others == ("a", "b") + assert (outcome.ok, outcome.reason) == (False, "Component would overlap") + + +class TestEveryKind: + @pytest.mark.parametrize( + "kind, attribute, reason", + [ + ("marker", "time", "nan is not a valid time"), + ("harmony", "time", "nan is not a valid time."), + ("mode", "time", "nan is not a valid time."), + ("pdf_marker", "time", "nan is not a valid time."), + ("beat", "time", "'nan' is not a valid time"), + ("hierarchy", "start", "'nan' is not a valid start"), + ("hierarchy", "end", "'nan' is not a valid end"), + ("hierarchy", "pre_start", "'nan' is not a valid pre start"), + ("hierarchy", "post_end", "'nan' is not a valid post end"), + ("range", "start", "nan is not a valid time"), + ("range", "end", "nan is not a valid time"), + ("range", "pre_start", "nan is not a valid time"), + ("range", "post_end", "nan is not a valid time"), + ], + ) + def test_a_time_of_nan_gets_the_importers_text(self, kind, attribute, reason): + assert check(kind, **{attribute: float("nan")}) == (False, reason) + + def test_tilia_creates_components_at_nan(self): + nan = float("nan") + + assert tilia_creates_point(nan) + assert tilia_creates_hierarchy(nan, 2.0, 1) + assert tilia_creates_hierarchy(1.0, nan, 1) + assert tilia_creates_range(nan, 2.0, ROW) + assert tilia_creates_range(1.0, 2.0, ROW, pre_start=nan) + + @pytest.mark.parametrize("value", [float("inf"), float("-inf"), "1.0", None, True]) + def test_a_time_that_isnt_a_finite_number(self, value): + assert check("marker", time=value) == ( + False, + f"{value} is not a valid time", + ) + + def test_a_missing_time(self): + assert rules.marker(MEDIA, [], {}) == (False, "None is not a valid time") + + @pytest.mark.parametrize("time", [0, 60]) + def test_times_given_as_integers(self, time): + assert check("marker", time=time) == OK + + @pytest.mark.parametrize( + "kind, attribute, reason", + [ + ("marker", "label", "3 is not a valid label"), + ("marker", "comments", "3 is not a valid comments"), + ("hierarchy", "label", "'3' is not a valid label"), + ("range", "comments", "3 is not a valid comments"), + ("harmony", "custom_text", "3 is not a valid custom text."), + ("mode", "comments", "3 is not a valid comments."), + ], + ) + def test_a_text_that_isnt_text(self, kind, attribute, reason): + assert check(kind, **{attribute: 3}) == (False, reason) + + @pytest.mark.parametrize("attribute", ["label", "comments"]) + def test_texts(self, attribute): + assert check("marker", **{attribute: ""}) == OK + assert check("marker", **{attribute: "Exposição 🎻"}) == OK + + def test_a_text_not_in_nfc_is_kept(self): + # Text is stored in NFC by whatever stores it: the reader, the + # writer and the CSV reader. + assert check("marker", label="Exposição") == OK + + +class TestColor: + @pytest.mark.parametrize( + "value", + [ + None, + "", + "#f0a", + "#FF00aa", + "#80FF00aa", + "#fff000aaa", + "#ffff0000aaaa", + "steelblue", + "SteelBlue", + "steel blue", + "\tred ", + "transparent", + ], + ) + def test_the_default_or_a_color_qt_reads(self, value): + assert rules.color(value) == OK + @pytest.mark.parametrize( + "value", + [ + "notacolor", + "#ff", + "#ffff", + "#fffffffffffffff", + "#ggg", + "#１２３", + " #fff", + "rgb(1, 2, 3)", + "red\n", + 0, + True, + ], + ) + def test_a_color_that_isnt_one(self, value): + assert rules.color(value) == (False, f"{value} is not a valid color.") -class TestPoint: + @pytest.mark.parametrize("kind", ["marker", "hierarchy", "range"]) + def test_each_kind_with_a_color_checks_it(self, kind): + assert check(kind, color="#f0a") == OK + assert check(kind, color="notacolor") == ( + False, + "notacolor is not a valid color.", + ) + + def test_tilia_creates_a_component_with_any_color(self): + assert tilia_creates_point(1.0, color="notacolor") + assert tilia_creates_hierarchy(1.0, 2.0, 1, color="notacolor") + assert tilia_creates_range(1.0, 2.0, ROW, color="notacolor") + + def test_colors_are_those_qt_reads(self): + qt = pytest.importorskip("PySide6.QtGui", reason="compares with Qt") + names = qt.QColor.colorNames() + samples = [ + *names, + *(name.upper() for name in names), + *(" ".join(name) for name in names), + "#fff", + "#FfF", + "#ffff", + "#fffff", + "#ff00ff00", + "#fffffffff", + "#ffffffffffff", + "#fffffffffffffff", + "#", + "#+12345", + "#12 345", + "#\tfff", + "#fff\t", + " #fff", + "fff", + "red\t", + "\tr\te\td\t", + "red\n", + "red ", + "red\x00", + "re\x00d", + "redΩ", + "redΩblah", + "Khaki", + "whİte", + "ſteelblue", + "ＲＥＤ", + "red\U0001f3bb", + "notacolor", + "a" * 300, + " " * 300 + "red", + " " * 250 + "red", + ] + + disagree = [ + sample + for sample in samples + if rules.color(sample).ok is not qt.QColor(sample).isValid() + ] + + assert disagree == [] + + +class TestPointKinds: + @pytest.mark.parametrize("kind", POINT_KINDS) @pytest.mark.parametrize("time", [0.0, 30.0, MEDIA]) - def test_a_time_from_0_to_the_media_length(self, time): - assert rules.point(time, MEDIA, set(), kind="marker") == OK + def test_a_time_from_0_to_the_media_length(self, kind, time): + assert check(kind, time=time) == OK - def test_a_time_beyond_the_media_length(self): - assert rules.point(60.5, MEDIA, set(), kind="marker") == ( + @pytest.mark.parametrize("kind", POINT_KINDS) + def test_a_negative_time(self, kind): + assert check(kind, time=-1.0) == ( + False, + "Time can't be negative. Got '-00:01.0'", + ) + + @pytest.mark.parametrize( + "time, shown", + [(-0.5, "-00:00.5"), (-61.5, "-01:01.5"), (-3725.0, "-1:02:05.0")], + ) + def test_a_negative_time_is_shown_with_a_minus_sign(self, time, shown): + assert check("marker", time=time, media_length=7200.0) == ( + False, + f"Time can't be negative. Got '{shown}'", + ) + + @pytest.mark.parametrize("kind", POINT_KINDS) + def test_a_time_beyond_the_media_length(self, kind): + assert check(kind, time=60.5) == ( False, "Time '01:00.5' is bigger than media time '01:00.0'", ) def test_times_of_an_hour_or_more_show_hours(self): - assert rules.point(3725.0, 3600.0, set(), kind="marker") == ( + assert check("marker", time=3725.0, media_length=3600.0) == ( False, "Time '1:02:05.0' is bigger than media time '1:00:00.0'", ) - def test_a_negative_time(self): - # TiLiA's format_media_time, as it is, shows -1 second as 59:59.0. - assert rules.point(-1.0, MEDIA, set(), kind="marker") == ( + def test_media_length_given_as_an_integer(self): + assert check("marker", time=61.0, media_length=60) == ( + False, + "Time '01:01.0' is bigger than media time '01:00.0'", + ) + + @pytest.mark.parametrize("kind", POINT_KINDS) + @pytest.mark.parametrize("time", [0.0, 1.0, MEDIA]) + def test_a_time_another_component_of_the_kind_has(self, kind, time): + same = component(kind, **{**VALID[kind], "time": time}) + others = [component(kind, **{**VALID[kind], "time": 30.5}), same] + + outcome = check(kind, others, time=time) + + assert outcome == ( False, - "Time can't be negative. Got '59:59.0'", + f"There is already a {POINT_NAMES[kind]} component at the selected position.", ) + assert outcome.others == (same.id,) + + @pytest.mark.parametrize("kind", POINT_KINDS) + def test_another_kind_at_the_same_time(self, kind): + other_kind = "mode" if kind == "harmony" else "harmony" + others = [component(other_kind, time=1.0)] + + assert check(kind, others, time=1.0) == OK + + def test_bounds_are_checked_before_other_components(self): + others = [component("marker", time=61.0)] + assert check("marker", others, time=61.0) == ( + False, + "Time '01:01.0' is bigger than media time '01:00.0'", + ) + + +class TestPdfMarker: + @pytest.mark.parametrize("page_number", [1, 2, PAGES]) + def test_a_page_the_pdf_has(self, page_number): + assert check("pdf_marker", page_number=page_number) == OK + + @pytest.mark.parametrize("page_number", [0, PAGES + 1, -1]) + def test_a_page_the_pdf_doesnt_have(self, page_number): + assert check("pdf_marker", page_number=page_number) == ( + False, + "The PDF has no page with that number.", + ) + + def test_a_pdf_that_cant_be_read_has_no_pages(self): + attrs = {"time": 1.0, "page_number": 1} + + assert rules.pdf_marker(MEDIA, [], attrs, page_count=0) == ( + False, + "The PDF has no page with that number.", + ) + + @pytest.mark.parametrize("page_number", [1.5, 1.0, "1", True, None]) + def test_a_page_number_that_isnt_an_integer(self, page_number): + assert check("pdf_marker", page_number=page_number) == ( + False, + f"{page_number} is not a valid page number.", + ) + + +class TestHarmony: @pytest.mark.parametrize( - "kind, name", + "attribute, values", [ - ("marker", "marker"), - ("harmony", "harmony"), - ("mode", "mode"), - ("beat", "beat"), - ("pdf_marker", "PDF page marker"), + ("step", [0, 6]), + ("applied_to", [0, 6]), + ("accidental", [-2, -1, 0, 1, 2]), + ("inversion", [0, 2]), + ("level", [1, 2]), + ("display_mode", ["letter", "roman", "custom"]), + ("custom_text_font_type", ["analytic", "normal"]), ], ) - def test_a_time_another_component_has(self, kind, name): - assert rules.point(1.0, MEDIA, {0.0, 1.0}, kind=kind) == ( - False, - f"There is already a {name} component at the selected position.", + def test_values_tilia_allows(self, attribute, values): + for value in values: + assert check("harmony", **{attribute: value}) == OK + + @pytest.mark.parametrize( + "attribute, value, reason", + [ + ("step", -1, "-1 is not a valid step."), + ("step", 7, "7 is not a valid step."), + ("step", 1.0, "1.0 is not a valid step."), + ("step", True, "True is not a valid step."), + ("applied_to", 7, "7 is not a valid applied to."), + ("accidental", 3, "3 is not a valid accidental."), + ("accidental", -3, "-3 is not a valid accidental."), + ("quality", "maj", "maj is not a valid quality."), + ("quality", None, "None is not a valid quality."), + ("inversion", -1, "-1 is not a valid inversion."), + ("inversion", 3, "3 is not a valid inversion."), + ("level", 0, "0 is not a valid level."), + ("level", 3, "3 is not a valid level."), + ("display_mode", "roman2", "roman2 is not a valid display mode."), + ( + "custom_text_font_type", + "bold", + "bold is not a valid custom text font type.", + ), + ], + ) + def test_values_tilia_refuses_when_it_sets_them(self, attribute, value, reason): + assert check("harmony", **{attribute: value}) == (False, reason) + assert tilia_creates_point(1.0, **{attribute: value}) + + def test_every_quality_music21_has(self, music21): + for quality in music21.harmony.CHORD_TYPES: + assert check("harmony", quality=quality) == OK + + def test_qualities_are_read_from_the_installed_music21(self, music21, monkeypatch): + monkeypatch.setitem( + music21.harmony.CHORD_TYPES, "made-up", ["1,2,3,4", ["made-up"]] ) - @pytest.mark.parametrize("time", [0.0, MEDIA]) - def test_a_time_another_component_has_at_the_edges(self, time): - assert rules.point(time, MEDIA, {time}, kind="marker")[0] is False + assert check("harmony", quality="made-up", inversion=3) == OK - def test_bounds_are_checked_before_other_components(self): - assert rules.point(61.0, MEDIA, {61.0}, kind="marker") == ( + @pytest.mark.parametrize( + "quality, inversions", + [("major", 2), ("dominant-seventh", 3), ("power", 0), ("Italian", 0)], + ) + def test_inversions_as_tilia_counts_them(self, quality, inversions): + assert check("harmony", quality=quality, inversion=inversions) == OK + assert check("harmony", quality=quality, inversion=inversions + 1) == ( False, - "Time '01:01.0' is bigger than media time '01:00.0'", + f"{inversions + 1} is not a valid inversion.", ) - def test_media_length_given_as_an_integer(self): - assert rules.point(61.0, 60, set(), kind="marker") == ( + def test_the_inversion_of_the_default_quality(self): + assert check("harmony", inversion=2) == OK + + def test_the_rules_in_order(self): + assert check("harmony", step=7, level=3) == (False, "7 is not a valid step.") + + +class TestMode: + @pytest.mark.parametrize( + "attribute, values", + [ + ("step", [0, 6]), + ("accidental", [-2, 2]), + ( + "type", + [ + "major", + "minor", + "dorian", + "phrygian", + "lydian", + "mixolydian", + "locrian", + ], + ), + ("level", [1, 2]), + ], + ) + def test_values_tilia_allows(self, attribute, values): + for value in values: + assert check("mode", **{attribute: value}) == OK + + @pytest.mark.parametrize( + "attribute, value, reason", + [ + ("step", 7, "7 is not a valid step."), + ("accidental", 3, "3 is not a valid accidental."), + ("type", "ionian", "ionian is not a valid type."), + ("type", "aeolian", "aeolian is not a valid type."), + ("type", "dorianx", "dorianx is not a valid type."), + ("level", 3, "3 is not a valid level."), + ], + ) + def test_values_tilia_refuses_when_it_sets_them(self, attribute, value, reason): + assert check("mode", **{attribute: value}) == (False, reason) + assert tilia_creates_point(1.0, **{attribute: value}) + + def test_types_are_read_from_the_installed_music21(self, music21, monkeypatch): + monkeypatch.setitem(music21.key.modeSharpsAlter, "made-up", 0) + + assert check("mode", type="made-up") == OK + + +class TestWithoutMusic21: + @pytest.mark.parametrize("kind", ["harmony", "mode"]) + def test_checking_fails_naming_the_install_command(self, kind, without_music21): + with pytest.raises(rules.CantCheck) as raised: + getattr(rules, kind)(MEDIA, [], {"time": 1.0}) + + assert INSTALL in str(raised.value) + assert raised.value.command == INSTALL + + @pytest.mark.parametrize("kind", ["harmony", "mode"]) + def test_the_other_rules_are_checked(self, kind, without_music21): + attrs = {"time": 1.0, "level": 3, "quality": "maj", "type": "dorianx"} + + assert getattr(rules, kind)(MEDIA, [], attrs) == ( False, - "Time '01:01.0' is bigger than media time '01:00.0'", + "3 is not a valid level.", ) + @pytest.mark.parametrize("kind", ["marker", "pdf_marker", "beat", "hierarchy"]) + def test_other_kinds_dont_need_it(self, kind, without_music21): + assert check(kind) == OK + -class TestSegment: +class TestHierarchy: @pytest.mark.parametrize("start, end", [(0.0, 1.0), (1.0, MEDIA), (0.0, MEDIA)]) def test_within_the_media_length(self, start, end): - assert rules.segment(start, end, (start, end, 1), set(), MEDIA) == OK + assert check("hierarchy", start=start, end=end) == OK def test_a_start_beyond_the_media_length(self): - assert rules.segment(61.0, 62.0, (61.0, 62.0, 1), set(), MEDIA) == ( + assert check("hierarchy", start=61.0, end=62.0) == ( False, "Start time '61.0' is bigger than media time '60.0'", ) def test_an_end_beyond_the_media_length(self): - assert rules.segment(1.0, 61.0, (1.0, 61.0, 1), set(), MEDIA) == ( + assert check("hierarchy", start=1.0, end=61.0) == ( False, "End time '61.0' is bigger than media time '60.0'", ) + @pytest.mark.parametrize("start", [-1.0, -0.5]) + def test_a_negative_start(self, start): + assert check("hierarchy", start=start) == ( + False, + f"Start time can't be negative. Got '{start}'", + ) + assert tilia_creates_hierarchy(start, 2.0, 1) + @pytest.mark.parametrize("end", [2.0, 1.0]) def test_an_end_not_after_its_start(self, end): - assert rules.segment(2.0, end, (2.0, end, 1), set(), MEDIA) == ( + assert check("hierarchy", start=2.0, end=end) == ( False, f"End time '{end}' should be bigger than start time '2.0'", ) - def test_the_position_of_another_component(self): - existing = {(0.0, 1.0, 1), (1.0, 2.0, 1)} + def test_numbers_given_as_integers_read_as_tilias_floats(self): + assert check("hierarchy", start=61, end=62, media_length=60) == ( + False, + "Start time '61.0' is bigger than media time '60.0'", + ) + + @pytest.mark.parametrize("level", [0, -1]) + def test_a_level_below_1(self, level): + assert check("hierarchy", level=level) == ( + False, + f"Level can't be below 1. Got '{level}'", + ) + assert tilia_creates_hierarchy(1.0, 2.0, level) + + @pytest.mark.parametrize("level", [1.5, 1.0, "1", True, None]) + def test_a_level_that_isnt_an_integer(self, level): + assert check("hierarchy", level=level) == ( + False, + f"'{level}' is not a valid level", + ) + + @pytest.mark.parametrize("pre_start", [None, 0.0, 0.5, 1.0]) + def test_a_pre_start_from_0_to_the_start(self, pre_start): + assert check("hierarchy", pre_start=pre_start) == OK + + def test_a_pre_start_below_0(self): + assert check("hierarchy", pre_start=-0.5) == ( + False, + "pre_start is outside media bounds.", + ) + assert tilia_creates_hierarchy(1.0, 2.0, 1, pre_start=-0.5) + + def test_a_pre_start_after_the_start(self): + assert check("hierarchy", pre_start=1.5) == ( + False, + "pre_start can't be after start time.", + ) + assert tilia_creates_hierarchy(1.0, 2.0, 1, pre_start=1.5) + + @pytest.mark.parametrize("post_end", [None, 2.0, 30.0, MEDIA]) + def test_a_post_end_from_the_end_to_the_media_length(self, post_end): + assert check("hierarchy", post_end=post_end) == OK + + def test_a_post_end_before_the_end(self): + assert check("hierarchy", post_end=1.5) == ( + False, + "post_end can't be before end time.", + ) + assert tilia_creates_hierarchy(1.0, 2.0, 1, post_end=1.5) + + def test_a_post_end_beyond_the_media_length(self): + assert check("hierarchy", post_end=60.5) == ( + False, + "post_end is outside media bounds.", + ) + assert tilia_creates_hierarchy(1.0, 2.0, 1, post_end=60.5) + + def test_the_position_of_another_hierarchy(self): + same = hierarchy(1.0, 2.0, 1) + others = [hierarchy(0.0, 1.0, 1), same] - assert rules.segment(1.0, 2.0, (1.0, 2.0, 1), existing, MEDIA) == ( + outcome = check("hierarchy", others, start=1.0, end=2.0, level=1) + + assert outcome == ( False, "There is already a hierarchy component at the selected position.", ) + assert outcome.others == (same.id,) def test_the_same_times_at_another_level(self): - assert rules.segment(1.0, 2.0, (1.0, 2.0, 2), {(1.0, 2.0, 1)}, MEDIA) == OK + others = [hierarchy(1.0, 2.0, 1)] + + assert check("hierarchy", others, start=1.0, end=2.0, level=2) == OK def test_times_are_checked_before_other_components(self): - position = (2.0, 1.0, 1) + others = [hierarchy(2.0, 1.0, 1)] - assert rules.segment(2.0, 1.0, position, {position}, MEDIA) == ( + assert check("hierarchy", others, start=2.0, end=1.0) == ( False, "End time '1.0' should be bigger than start time '2.0'", ) - def test_numbers_given_as_integers_read_as_tilias_floats(self): - assert rules.segment(61, 62, (61, 62, 1), set(), 60) == ( - False, - "Start time '61.0' is bigger than media time '60.0'", - ) + @pytest.mark.parametrize( + "existing, candidate", + [ + ((0, 10, 1), (5, 15, 1)), # they cross, at one level + ((5, 15, 1), (0, 10, 1)), + ((0, 10, 2), (5, 15, 1)), # they cross, at two levels + ((0, 10, 1), (5, 15, 2)), + ((0, 10, 1), (2, 5, 2)), # inside a lower one + ((0, 10, 1), (0, 5, 2)), + ((2, 5, 1), (0, 10, 1)), # around one at its level + ((2, 5, 2), (0, 10, 1)), # around a higher one + ((0, 10, 1), (2, 5, 1)), # inside one at its level + ], + ) + def test_hierarchies_that_dont_nest(self, existing, candidate): + other = hierarchy(*existing) + start, end, level = candidate + outcome = check("hierarchy", [other], start=start, end=end, level=level) -class TestRange: - ROWS = ["Ab12Cd", "Ef34Gh"] + assert outcome == (False, "Component would overlap with existing component.") + assert outcome.others == (other.id,) + assert tilia_creates_hierarchy(*candidate, existing=[existing]) - def check(self, start=1.0, end=2.0, row_id="Ab12Cd", pre_start=None, post_end=None): - return rules.range_(start, end, row_id, self.ROWS, pre_start, post_end, MEDIA) + @pytest.mark.parametrize( + "existing, candidate", + [ + ((0, 10, 2), (2, 5, 1)), # inside a higher one + ((0, 10, 2), (0, 10, 1)), # the same times, at another level + ((2, 5, 1), (0, 10, 2)), # around a lower one + ((0, 10, 1), (10, 20, 1)), # side by side + ((10, 20, 1), (0, 10, 2)), + ], + ) + def test_hierarchies_that_nest(self, existing, candidate): + start, end, level = candidate + others = [hierarchy(*existing)] + assert check("hierarchy", others, start=start, end=end, level=level) == OK + + def test_every_hierarchy_it_crosses(self): + crossed = [hierarchy(0, 10, 1), hierarchy(10, 20, 1)] + others = [*crossed, hierarchy(0, 30, 3)] + + outcome = check("hierarchy", others, start=5, end=15, level=2) + + assert outcome.others == tuple(h.id for h in crossed) + + def test_nesting_is_tilias_boundary_conflicts(self): + # Every pair of hierarchies with times from 0 to 4 and levels from 1 to 3. + spans = [ + (start, end, level) + for start, end in itertools.combinations(range(5), 2) + for level in (1, 2, 3) + ] + for existing, candidate in itertools.product(spans, repeat=2): + start, end, level = candidate + others = [hierarchy(*existing)] + + outcome = check("hierarchy", others, start=start, end=end, level=level) + + conflicts = tilia_boundary_conflicts([existing, candidate]) + assert outcome.ok is not bool(conflicts), (existing, candidate) + + def test_other_kinds_arent_compared(self): + others = [component("marker", time=1.0), range_in(ROW, 0.0, 5.0)] + + assert check("hierarchy", others) == OK + + +class TestRange: def test_a_range_in_a_row(self): - assert self.check() == OK + assert check("range") == OK @pytest.mark.parametrize( "start, end, pre_start, post_end", - [(0.0, MEDIA, None, None), (1.0, 2.0, 0.0, MEDIA), (0.0, MEDIA, 0.0, MEDIA)], + [ + (0.0, MEDIA, None, None), + (1.0, 2.0, 0.0, MEDIA), + (1.0, 2.0, 1.0, 2.0), + (0.0, MEDIA, 0.0, MEDIA), + ], ) def test_the_edges(self, start, end, pre_start, post_end): - assert self.check(start, end, pre_start=pre_start, post_end=post_end) == OK + attrs = {"start": start, "end": end, "pre_start": pre_start} + + assert check("range", **attrs, post_end=post_end) == OK @pytest.mark.parametrize("row_id", ["", None, "Zz99Zz"]) def test_a_row_the_timeline_doesnt_have(self, row_id): - assert self.check(row_id=row_id) == (False, "Invalid row ID.") + assert check("range", row_id=row_id) == (False, "Invalid row ID.") @pytest.mark.parametrize("end", [1.0, 0.5]) def test_an_end_not_after_its_start(self, end): - assert self.check(1.0, end) == (False, "Start time must be before end time.") + assert check("range", start=1.0, end=end) == ( + False, + "Start time must be before end time.", + ) @pytest.mark.parametrize("start, end", [(-0.5, 2.0), (1.0, 60.5)]) def test_outside_the_media(self, start, end): - assert self.check(start, end) == (False, "Range is outside media bounds.") + assert check("range", start=start, end=end) == ( + False, + "Range is outside media bounds.", + ) def test_a_pre_start_below_0(self): - assert self.check(pre_start=-0.5) == ( + assert check("range", pre_start=-0.5) == ( False, "pre_start is outside media bounds.", ) + def test_a_pre_start_after_the_start(self): + assert check("range", pre_start=1.5) == ( + False, + "pre_start can't be after start time.", + ) + assert tilia_creates_range(1.0, 2.0, ROW, pre_start=1.5) + def test_a_post_end_beyond_the_media_length(self): - assert self.check(post_end=60.5) == ( + assert check("range", post_end=60.5) == ( False, "post_end is outside media bounds.", ) + def test_a_post_end_before_the_end(self): + assert check("range", post_end=1.5) == ( + False, + "post_end can't be before end time.", + ) + assert tilia_creates_range(1.0, 2.0, ROW, post_end=1.5) + def test_the_row_is_checked_first(self): - assert self.check(2.0, 1.0, row_id="Zz99Zz") == (False, "Invalid row ID.") + assert check("range", start=2.0, end=1.0, row_id="Zz99Zz") == ( + False, + "Invalid row ID.", + ) + def test_ranges_in_a_row_may_overlap(self): + others = [range_in(ROW, 0.0, 5.0), range_in(ROW, 1.0, 2.0)] -class TestPage: - @pytest.mark.parametrize("page_number", [1, 2, 3]) - def test_a_page_the_pdf_has(self, page_number): - assert rules.page(page_number, 3) == OK + assert check("range", others, start=1.0, end=2.0) == OK + + +class TestJoinedRight: + def test_joined_to_the_next_range_in_its_row(self): + following = range_in(ROW, 2.0, 3.0) + others = [range_in(ROW, 0.0, 1.0), range_in(ROW, 4.0, 5.0), following] + + assert check("range", others, joined_right=following.id) == OK + + def test_the_next_ranges_share_their_start(self): + following = [range_in(ROW, 2.0, 3.0), range_in(ROW, 2.0, 4.0)] + + for joined in following: + assert check("range", following, joined_right=joined.id) == OK + + def test_a_range_with_the_same_start_doesnt_come_next(self): + following = range_in(ROW, 2.0, 3.0) + others = [range_in(ROW, 1.0, 1.5), following] + + assert check("range", others, joined_right=following.id) == OK + + def test_the_last_range_of_its_row(self): + others = [range_in(ROW, 0.0, 1.0), range_in("Ef34Gh", 2.0, 3.0)] + + assert check("range", others, joined_right=others[1].id) == ( + False, + "joined_with_next=true on last range of row 'Strings';" + " no next neighbor to join with", + ) + + def test_the_next_range_doesnt_start_at_its_end(self): + following = range_in(ROW, 2.5, 3.0) + others = [following, range_in(ROW, 4.0, 5.0)] + + outcome = check("range", others, joined_right=following.id) + + assert outcome == ( + False, + "joined_with_next=true but next range on row 'Strings' starts at 2.5" + " (must equal 2.0 for ranges to join)", + ) + assert outcome.others == (following.id,) + + def test_joined_to_a_range_that_isnt_the_next(self): + between = range_in(ROW, 1.5, 1.8) + later = range_in(ROW, 2.0, 3.0) + + outcome = check("range", [between, later], end=1.5, joined_right=later.id) + + assert outcome == ( + False, + "joined_with_next=true but the range it joins isn't the next range" + " on row 'Strings'", + ) + assert outcome.others == (between.id,) + + def test_joined_to_a_range_in_another_row(self): + following = range_in(ROW, 2.0, 3.0) + elsewhere = range_in("Ef34Gh", 2.0, 3.0) + + outcome = check("range", [following, elsewhere], joined_right=elsewhere.id) + + assert outcome == ( + False, + "joined_with_next=true but the range it joins isn't the next range" + " on row 'Strings'", + ) + + +class TestRows: + def test_rows(self): + assert rules.range_rows(ROWS) == OK + assert rules.range_rows([{"id": "000000", "name": "", "color": "#f0a"}]) == OK @pytest.mark.parametrize( - "page_number, page_total", [(0, 3), (4, 3), (-1, 3), (1, 0)] + "row_id", ["Ab12C", "Ab12Cd7", "Ab-2Cd", "Ab12Cé", 123456, None] ) - def test_a_page_the_pdf_doesnt_have(self, page_number, page_total): - assert rules.page(page_number, page_total) == ( + def test_an_id_that_isnt_six_letters_and_digits(self, row_id): + assert rules.range_rows([{"id": row_id, "name": "Strings"}]) == ( False, - "The PDF has no page with that number.", + f"{row_id} is not a valid row id", + ) + + def test_an_id_two_rows_have(self): + rows = [{"id": ROW, "name": "Strings"}, {"id": ROW, "name": "Winds"}] + + assert rules.range_rows(rows) == (False, f"{ROW} is the id of two rows") + + @pytest.mark.parametrize("name", [3, None]) + def test_a_name_that_isnt_text(self, name): + assert rules.range_rows([{"id": ROW, "name": name}]) == ( + False, + f"{name} is not a valid row name", + ) + + @pytest.mark.parametrize("color", [None, "", "steelblue"]) + def test_a_color(self, color): + assert rules.range_rows([{"id": ROW, "name": "Strings", "color": color}]) == OK + + def test_a_color_that_isnt_one(self): + # TiLiA's own text, for a row's color. + assert rules.range_rows( + [{"id": ROW, "name": "Strings", "color": "notacolor"}] + ) == ( + False, + "'notacolor' is not a valid color.", ) + + +class TestNotValid: + @pytest.mark.parametrize( + "kind, attribute, value, text", + [ + ("marker", "time", "abc", "abc is not a valid time"), + ("hierarchy", "start", "abc", "'abc' is not a valid start"), + ("hierarchy", "post_end", "abc", "'abc' is not a valid post end"), + ("range", "end", "abc", "abc is not a valid time"), + ("range", "label", 3, "3 is not a valid label"), + ("harmony", "display_mode", "abc", "abc is not a valid display mode."), + ("mode", "type", "abc", "abc is not a valid type."), + ("pdf_marker", "page_number", "abc", "abc is not a valid page number."), + ("beat", "time", "abc", "'abc' is not a valid time"), + ], + ) + def test_as_each_kinds_importer_words_a_value_it_cant_read( + self, kind, attribute, value, text + ): + assert rules.not_valid(kind, attribute, value) == text From 9f460774402c6c26188d1a1a04aeb3f93da50748 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:00:59 +0200 Subject: [PATCH 04/37] feat(core): one form for the reason a value isn't one Every reason for a value that isn't one, whatever the kind, now reads " is not a valid .": a time of nan, a level or page number that isn't an integer, a text that isn't text, a color, a harmony value, a row's id and name. not_valid(attribute, value) gives it, for the importers to give the same for a value they can't read. Two rows with the same id give "There is already a row with the id Ab12Cd.". Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 87 +++++++++------------ packages/tilia-core/tests/test_rules.py | 74 +++++++++--------- 2 files changed, 73 insertions(+), 88 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 0f3cbfcef..7adfa5d47 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -11,7 +11,8 @@ length, the timeline's other components (not the one checked) and the attributes of the component to check, and gives the first rule they break: `(False, reason)`, or `(True, "")` when they break none. The reasons are -TiLiA's, word for word, where TiLiA has one. +TiLiA's, word for word, where TiLiA has one. A value that isn't one always +reads ` is not a valid .`, whatever the kind. A harmony's quality and inversion and a mode's type are checked against the installed music21, as TiLiA checks them, so checking a harmony or a mode needs @@ -60,18 +61,6 @@ _SEGMENT_TIMES = ("start", "end", "pre_start", "post_end") _UNSET_TIMES = ("pre_start", "post_end") # None: they follow the start and the end -# How each kind's importer words a value it can't read (tilia/parsers/csv/), -# leaving out what names the row. -_NOT_VALID = { - "marker": "{value} is not a valid {attribute}", - "hierarchy": "'{value}' is not a valid {attribute}", - "range": "{value} is not a valid {attribute}", - "harmony": "{value} is not a valid {attribute}.", - "mode": "{value} is not a valid {attribute}.", - "pdf_marker": "{value} is not a valid {attribute}.", - "beat": "'{value}' is not a valid {attribute}", -} - _ROW_ID_LENGTH = 6 _ROW_ID_CHARACTERS = frozenset(string.ascii_letters + string.digits) @@ -191,13 +180,13 @@ def harmony( music21 = _music21() broken = ( _point("harmony", media_length, others, attrs) - or _one_of("harmony", attrs, "step", _STEPS) - or _one_of("harmony", attrs, "applied_to", _STEPS) - or _one_of("harmony", attrs, "accidental", HARMONY_ACCIDENTALS) + or _one_of(attrs, "step", _STEPS) + or _one_of(attrs, "applied_to", _STEPS) + or _one_of(attrs, "accidental", HARMONY_ACCIDENTALS) or (_quality_and_inversion(music21, attrs) if music21 else None) - or _one_of("harmony", attrs, "level", _LEVELS) - or _one_of("harmony", attrs, "display_mode", HARMONY_DISPLAY_MODES) - or _one_of("harmony", attrs, "custom_text_font_type", FONT_TYPES) + or _one_of(attrs, "level", _LEVELS) + or _one_of(attrs, "display_mode", HARMONY_DISPLAY_MODES) + or _one_of(attrs, "custom_text_font_type", FONT_TYPES) ) return _checked("harmony", broken, music21) @@ -209,10 +198,10 @@ def mode( music21 = _music21() broken = ( _point("mode", media_length, others, attrs) - or _one_of("mode", attrs, "step", _STEPS) - or _one_of("mode", attrs, "accidental", HARMONY_ACCIDENTALS) + or _one_of(attrs, "step", _STEPS) + or _one_of(attrs, "accidental", HARMONY_ACCIDENTALS) or (_mode_type(music21, attrs) if music21 else None) - or _one_of("mode", attrs, "level", _LEVELS) + or _one_of(attrs, "level", _LEVELS) ) return _checked("mode", broken, music21) @@ -222,7 +211,7 @@ def hierarchy( ) -> Outcome: """A hierarchy. Hierarchies nest: one that overlaps another lies inside it, at a lower level.""" - broken = _every_kind("hierarchy", attrs, _SEGMENT_TIMES) + broken = _every_kind(attrs, _SEGMENT_TIMES) if broken: return broken start, end, level = attrs["start"], attrs["end"], attrs.get("level") @@ -249,7 +238,7 @@ def range_( Ranges in a row may overlap. """ - broken = _every_kind("range", attrs, _SEGMENT_TIMES) + broken = _every_kind(attrs, _SEGMENT_TIMES) if broken: return broken start, end = attrs["start"], attrs["end"] @@ -270,16 +259,16 @@ def range_rows(rows: Sequence[Mapping[str, Any]]) -> Outcome: for row in rows: row_id = row.get("id") if not _is_row_id(row_id): - return _broken(f"{row_id} is not a valid row id") + return _broken(not_valid("row_id", row_id)) if row_id in ids: - return _broken(f"{row_id} is the id of two rows") + return _broken(f"There is already a row with the id {row_id}.") ids.add(row_id) name = row.get("name") if not isinstance(name, str): - return _broken(f"{name} is not a valid row name") - if not color(row.get("color")).ok: - # TiLiA's text for a row's color, which isn't the components' one. - return _broken(f"'{row.get('color')}' is not a valid color.") + return _broken(not_valid("row_name", name)) + outcome = color(row.get("color")) + if not outcome.ok: + return outcome return OK @@ -293,18 +282,16 @@ def color(value: Any) -> Outcome: return OK if isinstance(value, str) and _qt_reads(value): return OK - return _broken(f"{value} is not a valid color.") + return _broken(not_valid("color", value)) -def not_valid(kind: str, attribute: str, value: Any) -> str: - """How the importer of `kind` words a value of `attribute` it can't read. +def not_valid(attribute: str, value: Any) -> str: + """The reason for a value of `attribute` that isn't one, for every kind. - The rules give the same text for a value that isn't one, such as a time of - nan. An importer gives it for a value it can't parse, so the two read alike. + The rules give it for a value they refuse as not being one, such as a time of + nan, and the importers for a value they can't read, so the two read alike. """ - if kind == "range" and attribute in _SEGMENT_TIMES: - attribute = "time" # as the range importer calls them - return _NOT_VALID[kind].format(value=value, attribute=attribute.replace("_", " ")) + return f"{value} is not a valid {attribute.replace('_', ' ')}." def _point( @@ -314,7 +301,7 @@ def _point( attrs: Mapping[str, Any], ) -> Outcome | None: """The rules for every kind, the time's bounds, and no other at the same time.""" - broken = _every_kind(kind, attrs, ("time",)) + broken = _every_kind(attrs, ("time",)) if broken: return broken time = attrs["time"] @@ -339,19 +326,17 @@ def _point( return None -def _every_kind( - kind: str, attrs: Mapping[str, Any], times: Iterable[str] -) -> Outcome | None: +def _every_kind(attrs: Mapping[str, Any], times: Iterable[str]) -> Outcome | None: """Times are finite numbers, texts are text, and a color is one.""" for name in times: value = attrs.get(name) if value is None and name in _UNSET_TIMES: continue if not _is_number(value): - return _broken(not_valid(kind, name, value)) + return _broken(not_valid(name, value)) for name in _TEXTS: if name in attrs and not isinstance(attrs[name], str): - return _broken(not_valid(kind, name, attrs[name])) + return _broken(not_valid(name, attrs[name])) if "color" in attrs: outcome = color(attrs["color"]) if not outcome.ok: @@ -361,14 +346,14 @@ def _every_kind( def _page(page_number: Any, page_count: int) -> Outcome | None: if not _is_integer(page_number): - return _broken(not_valid("pdf_marker", "page_number", page_number)) + return _broken(not_valid("page_number", page_number)) if not 1 <= page_number <= page_count: return _broken("The PDF has no page with that number.") return None def _one_of( - kind: str, attrs: Mapping[str, Any], name: str, allowed: Collection[int | str] + attrs: Mapping[str, Any], name: str, allowed: Collection[int | str] ) -> Outcome | None: """`name`, when the component has it, is one of `allowed`, as it is: 1.0 isn't 1.""" if name not in attrs: @@ -376,7 +361,7 @@ def _one_of( value = attrs[name] if (_is_integer(value) or isinstance(value, str)) and value in allowed: return None - return _broken(not_valid(kind, name, value)) + return _broken(not_valid(name, value)) def _music21() -> ModuleType | None: @@ -403,10 +388,10 @@ def _quality_and_inversion( """The quality is one of music21's `CHORD_TYPES`, and the inversion one it has.""" quality = attrs.get("quality", "major") if not (isinstance(quality, str) and quality in music21.harmony.CHORD_TYPES): - return _broken(not_valid("harmony", "quality", quality)) + return _broken(not_valid("quality", quality)) inversion = attrs.get("inversion", 0) if not (_is_integer(inversion) and 0 <= inversion <= _inversions(music21, quality)): - return _broken(not_valid("harmony", "inversion", inversion)) + return _broken(not_valid("inversion", inversion)) return None @@ -423,7 +408,7 @@ def _mode_type(music21: ModuleType, attrs: Mapping[str, Any]) -> Outcome | None: types = [ name for name in music21.key.modeSharpsAlter if name not in _NOT_MODE_TYPES ] - return _one_of("mode", attrs, "type", types) + return _one_of(attrs, "type", types) def _hierarchy_times(start: float, end: float, media_length: float) -> Outcome | None: @@ -444,7 +429,7 @@ def _hierarchy_times(start: float, end: float, media_length: float) -> Outcome | def _hierarchy_level(level: Any) -> Outcome | None: if not _is_integer(level): - return _broken(not_valid("hierarchy", "level", level)) + return _broken(not_valid("level", level)) if level < 1: return _broken(f"Level can't be below 1. Got '{level}'") return None diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index bb66c9ab9..846120bcd 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -157,22 +157,22 @@ class TestEveryKind: @pytest.mark.parametrize( "kind, attribute, reason", [ - ("marker", "time", "nan is not a valid time"), + ("marker", "time", "nan is not a valid time."), ("harmony", "time", "nan is not a valid time."), ("mode", "time", "nan is not a valid time."), ("pdf_marker", "time", "nan is not a valid time."), - ("beat", "time", "'nan' is not a valid time"), - ("hierarchy", "start", "'nan' is not a valid start"), - ("hierarchy", "end", "'nan' is not a valid end"), - ("hierarchy", "pre_start", "'nan' is not a valid pre start"), - ("hierarchy", "post_end", "'nan' is not a valid post end"), - ("range", "start", "nan is not a valid time"), - ("range", "end", "nan is not a valid time"), - ("range", "pre_start", "nan is not a valid time"), - ("range", "post_end", "nan is not a valid time"), + ("beat", "time", "nan is not a valid time."), + ("hierarchy", "start", "nan is not a valid start."), + ("hierarchy", "end", "nan is not a valid end."), + ("hierarchy", "pre_start", "nan is not a valid pre start."), + ("hierarchy", "post_end", "nan is not a valid post end."), + ("range", "start", "nan is not a valid start."), + ("range", "end", "nan is not a valid end."), + ("range", "pre_start", "nan is not a valid pre start."), + ("range", "post_end", "nan is not a valid post end."), ], ) - def test_a_time_of_nan_gets_the_importers_text(self, kind, attribute, reason): + def test_a_time_of_nan(self, kind, attribute, reason): assert check(kind, **{attribute: float("nan")}) == (False, reason) def test_tilia_creates_components_at_nan(self): @@ -188,11 +188,11 @@ def test_tilia_creates_components_at_nan(self): def test_a_time_that_isnt_a_finite_number(self, value): assert check("marker", time=value) == ( False, - f"{value} is not a valid time", + f"{value} is not a valid time.", ) def test_a_missing_time(self): - assert rules.marker(MEDIA, [], {}) == (False, "None is not a valid time") + assert rules.marker(MEDIA, [], {}) == (False, "None is not a valid time.") @pytest.mark.parametrize("time", [0, 60]) def test_times_given_as_integers(self, time): @@ -201,10 +201,10 @@ def test_times_given_as_integers(self, time): @pytest.mark.parametrize( "kind, attribute, reason", [ - ("marker", "label", "3 is not a valid label"), - ("marker", "comments", "3 is not a valid comments"), - ("hierarchy", "label", "'3' is not a valid label"), - ("range", "comments", "3 is not a valid comments"), + ("marker", "label", "3 is not a valid label."), + ("marker", "comments", "3 is not a valid comments."), + ("hierarchy", "label", "3 is not a valid label."), + ("range", "comments", "3 is not a valid comments."), ("harmony", "custom_text", "3 is not a valid custom text."), ("mode", "comments", "3 is not a valid comments."), ], @@ -619,7 +619,7 @@ def test_a_level_below_1(self, level): def test_a_level_that_isnt_an_integer(self, level): assert check("hierarchy", level=level) == ( False, - f"'{level}' is not a valid level", + f"{level} is not a valid level.", ) @pytest.mark.parametrize("pre_start", [None, 0.0, 0.5, 1.0]) @@ -905,19 +905,22 @@ def test_rows(self): def test_an_id_that_isnt_six_letters_and_digits(self, row_id): assert rules.range_rows([{"id": row_id, "name": "Strings"}]) == ( False, - f"{row_id} is not a valid row id", + f"{row_id} is not a valid row id.", ) def test_an_id_two_rows_have(self): rows = [{"id": ROW, "name": "Strings"}, {"id": ROW, "name": "Winds"}] - assert rules.range_rows(rows) == (False, f"{ROW} is the id of two rows") + assert rules.range_rows(rows) == ( + False, + f"There is already a row with the id {ROW}.", + ) @pytest.mark.parametrize("name", [3, None]) def test_a_name_that_isnt_text(self, name): assert rules.range_rows([{"id": ROW, "name": name}]) == ( False, - f"{name} is not a valid row name", + f"{name} is not a valid row name.", ) @pytest.mark.parametrize("color", [None, "", "steelblue"]) @@ -925,31 +928,28 @@ def test_a_color(self, color): assert rules.range_rows([{"id": ROW, "name": "Strings", "color": color}]) == OK def test_a_color_that_isnt_one(self): - # TiLiA's own text, for a row's color. assert rules.range_rows( [{"id": ROW, "name": "Strings", "color": "notacolor"}] ) == ( False, - "'notacolor' is not a valid color.", + "notacolor is not a valid color.", ) class TestNotValid: @pytest.mark.parametrize( - "kind, attribute, value, text", + "attribute, value, text", [ - ("marker", "time", "abc", "abc is not a valid time"), - ("hierarchy", "start", "abc", "'abc' is not a valid start"), - ("hierarchy", "post_end", "abc", "'abc' is not a valid post end"), - ("range", "end", "abc", "abc is not a valid time"), - ("range", "label", 3, "3 is not a valid label"), - ("harmony", "display_mode", "abc", "abc is not a valid display mode."), - ("mode", "type", "abc", "abc is not a valid type."), - ("pdf_marker", "page_number", "abc", "abc is not a valid page number."), - ("beat", "time", "abc", "'abc' is not a valid time"), + ("time", "abc", "abc is not a valid time."), + ("post_end", "abc", "abc is not a valid post end."), + ("label", 3, "3 is not a valid label."), + ("page_number", 1.5, "1.5 is not a valid page number."), + ( + "custom_text_font_type", + "bold", + "bold is not a valid custom text font type.", + ), ], ) - def test_as_each_kinds_importer_words_a_value_it_cant_read( - self, kind, attribute, value, text - ): - assert rules.not_valid(kind, attribute, value) == text + def test_one_text_for_a_value_that_isnt_one(self, attribute, value, text): + assert rules.not_valid(attribute, value) == text From 12d75875acc5a423604f998bd29fbf576ffbd8dc Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Wed, 14 Oct 2026 15:13:35 +0200 Subject: [PATCH 05/37] feat(core): an outcome is true when no rule is broken `if rules.marker(...)` was always true, since an Outcome is a non-empty tuple. It now reads as "keeps to the rules": bool(outcome) is outcome.ok, and it still unpacks as (ok, reason). The rules were chained with `or`, which relied on a broken outcome being true; they are now checked in order by _first, each only once the ones before it are kept. The nesting test now covers levels from 1 to 8. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 107 ++++++++++++-------- packages/tilia-core/tests/test_rules.py | 13 ++- 2 files changed, 73 insertions(+), 47 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 7adfa5d47..3260bf324 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -26,7 +26,7 @@ import importlib.util import math import string -from collections.abc import Collection, Iterable, Mapping, Sequence +from collections.abc import Callable, Collection, Iterable, Mapping, Sequence from types import ModuleType from typing import TYPE_CHECKING, Any @@ -103,9 +103,10 @@ class Outcome(tuple[bool, str]): """What checking the rules gives: `(True, "")`, or `(False, reason)`. - `others` are the ids of the other components a broken rule is about: those - at the same time or place, the hierarchies it crosses, the ranges it should - join. + It is true when no rule is broken, so `if rules.marker(...):` reads as "the + marker keeps to the rules"; it also unpacks as `ok, reason`. `others` are + the ids of the other components a broken rule is about: those at the same + time or place, the hierarchies it crosses, the ranges it should join. """ others: tuple[str, ...] @@ -115,6 +116,9 @@ def __new__(cls, ok: bool, reason: str = "", others: Iterable[str] = ()) -> Outc outcome.others = tuple(others) return outcome + def __bool__(self) -> bool: + return self[0] + @property def ok(self) -> bool: return self[0] @@ -140,22 +144,38 @@ def __init__(self, kind: str) -> None: self.kind = kind -# The private functions give the first rule they find broken, or None, so that -# `or` checks them in order. +# The private functions give the first rule they find broken, or None. `_first` +# checks them in order, each only once the ones before it are kept: an outcome's +# truth is whether it is kept, so `or` can't chain them. + +Check = Callable[[], "Outcome | None"] + + +def _first_broken(*checks: Check) -> Outcome | None: + for check in checks: + broken = check() + if broken is not None: + return broken + return None + + +def _first(*checks: Check) -> Outcome: + broken = _first_broken(*checks) + return OK if broken is None else broken def marker( media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] ) -> Outcome: """A marker.""" - return _point("marker", media_length, others, attrs) or OK + return _first(lambda: _point("marker", media_length, others, attrs)) def beat( media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] ) -> Outcome: """A beat. Its measure mark and beat unit follow the measure table's rules.""" - return _point("beat", media_length, others, attrs) or OK + return _first(lambda: _point("beat", media_length, others, attrs)) def pdf_marker( @@ -166,10 +186,9 @@ def pdf_marker( page_count: int, ) -> Outcome: """A PDF marker, in a PDF of `page_count` pages: 0 for a PDF that can't be read.""" - return ( - _point("pdf_marker", media_length, others, attrs) - or _page(attrs.get("page_number"), page_count) - or OK + return _first( + lambda: _point("pdf_marker", media_length, others, attrs), + lambda: _page(attrs.get("page_number"), page_count), ) @@ -178,15 +197,15 @@ def harmony( ) -> Outcome: """A harmony. Without music21, raises `CantCheck` when no other rule is broken.""" music21 = _music21() - broken = ( - _point("harmony", media_length, others, attrs) - or _one_of(attrs, "step", _STEPS) - or _one_of(attrs, "applied_to", _STEPS) - or _one_of(attrs, "accidental", HARMONY_ACCIDENTALS) - or (_quality_and_inversion(music21, attrs) if music21 else None) - or _one_of(attrs, "level", _LEVELS) - or _one_of(attrs, "display_mode", HARMONY_DISPLAY_MODES) - or _one_of(attrs, "custom_text_font_type", FONT_TYPES) + broken = _first_broken( + lambda: _point("harmony", media_length, others, attrs), + lambda: _one_of(attrs, "step", _STEPS), + lambda: _one_of(attrs, "applied_to", _STEPS), + lambda: _one_of(attrs, "accidental", HARMONY_ACCIDENTALS), + lambda: _quality_and_inversion(music21, attrs) if music21 else None, + lambda: _one_of(attrs, "level", _LEVELS), + lambda: _one_of(attrs, "display_mode", HARMONY_DISPLAY_MODES), + lambda: _one_of(attrs, "custom_text_font_type", FONT_TYPES), ) return _checked("harmony", broken, music21) @@ -196,12 +215,12 @@ def mode( ) -> Outcome: """A key. Without music21, raises `CantCheck` when no other rule is broken.""" music21 = _music21() - broken = ( - _point("mode", media_length, others, attrs) - or _one_of(attrs, "step", _STEPS) - or _one_of(attrs, "accidental", HARMONY_ACCIDENTALS) - or (_mode_type(music21, attrs) if music21 else None) - or _one_of(attrs, "level", _LEVELS) + broken = _first_broken( + lambda: _point("mode", media_length, others, attrs), + lambda: _one_of(attrs, "step", _STEPS), + lambda: _one_of(attrs, "accidental", HARMONY_ACCIDENTALS), + lambda: _mode_type(music21, attrs) if music21 else None, + lambda: _one_of(attrs, "level", _LEVELS), ) return _checked("mode", broken, music21) @@ -212,18 +231,17 @@ def hierarchy( """A hierarchy. Hierarchies nest: one that overlaps another lies inside it, at a lower level.""" broken = _every_kind(attrs, _SEGMENT_TIMES) - if broken: + if broken is not None: return broken start, end, level = attrs["start"], attrs["end"], attrs.get("level") hierarchies = [other for other in others if other.kind == "hierarchy"] - return ( - _hierarchy_times(start, end, media_length) - or _hierarchy_level(level) - or _pre_start(attrs.get("pre_start"), start) - or _post_end(attrs.get("post_end"), end, media_length) - or _same_place(start, end, level, hierarchies) - or _crossed(start, end, level, hierarchies) - or OK + return _first( + lambda: _hierarchy_times(start, end, media_length), + lambda: _hierarchy_level(level), + lambda: _pre_start(attrs.get("pre_start"), start), + lambda: _post_end(attrs.get("post_end"), end, media_length), + lambda: _same_place(start, end, level, hierarchies), + lambda: _crossed(start, end, level, hierarchies), ) @@ -239,16 +257,15 @@ def range_( Ranges in a row may overlap. """ broken = _every_kind(attrs, _SEGMENT_TIMES) - if broken: + if broken is not None: return broken start, end = attrs["start"], attrs["end"] row_ids = [row.get("id") for row in rows] - return ( - _range_place(attrs.get("row_id"), row_ids, start, end, media_length) - or _pre_start(attrs.get("pre_start"), start) - or _post_end(attrs.get("post_end"), end, media_length) - or _joined(attrs, others, rows) - or OK + return _first( + lambda: _range_place(attrs.get("row_id"), row_ids, start, end, media_length), + lambda: _pre_start(attrs.get("pre_start"), start), + lambda: _post_end(attrs.get("post_end"), end, media_length), + lambda: _joined(attrs, others, rows), ) @@ -302,7 +319,7 @@ def _point( ) -> Outcome | None: """The rules for every kind, the time's bounds, and no other at the same time.""" broken = _every_kind(attrs, ("time",)) - if broken: + if broken is not None: return broken time = attrs["time"] if time < 0: @@ -375,7 +392,7 @@ def _music21() -> ModuleType | None: def _checked(kind: str, broken: Outcome | None, music21: ModuleType | None) -> Outcome: - if broken: + if broken is not None: return broken if music21 is None: raise CantCheck(kind) diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 846120bcd..94656deaa 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -140,6 +140,15 @@ def test_a_kept_rule_is_true_and_no_reason(self): assert rules.OK == (True, "") assert rules.OK.others == () + def test_an_outcome_is_true_only_when_no_rule_is_broken(self): + assert rules.OK + assert not rules.Outcome(False, "Invalid row ID.") + + def test_if_reads_as_keeping_to_the_rules(self): + assert rules.marker(60.0, [], {"time": 1.0}) + assert not rules.marker(60.0, [], {"time": -1.0}) + assert not rules.hierarchy(60.0, [], {"start": 0.0, "end": 1.0, "level": 0}) + def test_a_broken_rule_unpacks_as_its_reason(self): ok, reason = rules.Outcome(False, "Invalid row ID.") @@ -732,11 +741,11 @@ def test_every_hierarchy_it_crosses(self): assert outcome.others == tuple(h.id for h in crossed) def test_nesting_is_tilias_boundary_conflicts(self): - # Every pair of hierarchies with times from 0 to 4 and levels from 1 to 3. + # Every pair of hierarchies with times from 0 to 4 and levels from 1 to 8. spans = [ (start, end, level) for start, end in itertools.combinations(range(5), 2) - for level in (1, 2, 3) + for level in range(1, 9) ] for existing, candidate in itertools.product(spans, repeat=2): start, end, level = candidate From b0ada5343a125657406314b0f816ce37f17faa21 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:31:05 +0200 Subject: [PATCH 06/37] feat(core): take the colour names from webcolors The 147 colour names were written out in rules.py: a copy of CSS 3's (SVG 1.0's), which is what QColor.colorNames() gives, with "transparent". They now come from webcolors.names("css3"), and the core adds only "transparent". webcolors (pure Python, BSD-3-Clause, no dependencies; >=24.8.0, the first with names()) becomes tilia-core's second dependency; music21, and so the app, already install it. A test checks that every CSS 3 name and "transparent" are colours, and CSS 4's "rebeccapurple", which Qt doesn't read, isn't. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/pyproject.toml | 2 +- packages/tilia-core/src/tilia_core/rules.py | 36 +++------------------ packages/tilia-core/tests/test_rules.py | 7 ++++ 3 files changed, 13 insertions(+), 32 deletions(-) diff --git a/packages/tilia-core/pyproject.toml b/packages/tilia-core/pyproject.toml index c0ba73bd5..2436a1760 100644 --- a/packages/tilia-core/pyproject.toml +++ b/packages/tilia-core/pyproject.toml @@ -8,7 +8,7 @@ version = "0.0.2" description = "TiLiA's file format and corpus tools, without a user interface" readme = "README.md" requires-python = ">=3.10" -dependencies = ["pypdf>=6.0.0"] +dependencies = ["pypdf>=6.0.0", "webcolors>=24.8.0"] license = "GPL-3.0-or-later" license-files = ["LICENSE"] authors = [ diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 3260bf324..120d5653b 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -30,6 +30,8 @@ from types import ModuleType from typing import TYPE_CHECKING, Any +import webcolors + if TYPE_CHECKING: from tilia_core import tla @@ -65,39 +67,11 @@ _ROW_ID_CHARACTERS = frozenset(string.ascii_letters + string.digits) # The colors Qt reads: #RGB, #RRGGBB, #AARRGGBB, #RRRGGGBBB and #RRRRGGGGBBBB, -# and the SVG color names with "transparent" (`QColor.colorNames()`). +# and the names in `QColor.colorNames()`, which are CSS 3's (SVG 1.0's) and +# "transparent". CSS 3's come from webcolors, so the core keeps no copy of them. _HEX_LENGTHS = (3, 6, 8, 9, 12) _HEX_DIGITS = frozenset(string.hexdigits) -_COLOR_NAMES = frozenset( - { - "aliceblue", "antiquewhite", "aqua", "aquamarine", "azure", "beige", "bisque", - "black", "blanchedalmond", "blue", "blueviolet", "brown", "burlywood", - "cadetblue", "chartreuse", "chocolate", "coral", "cornflowerblue", "cornsilk", - "crimson", "cyan", "darkblue", "darkcyan", "darkgoldenrod", "darkgray", - "darkgreen", "darkgrey", "darkkhaki", "darkmagenta", "darkolivegreen", - "darkorange", "darkorchid", "darkred", "darksalmon", "darkseagreen", - "darkslateblue", "darkslategray", "darkslategrey", "darkturquoise", - "darkviolet", "deeppink", "deepskyblue", "dimgray", "dimgrey", "dodgerblue", - "firebrick", "floralwhite", "forestgreen", "fuchsia", "gainsboro", "ghostwhite", - "gold", "goldenrod", "gray", "green", "greenyellow", "grey", "honeydew", - "hotpink", "indianred", "indigo", "ivory", "khaki", "lavender", "lavenderblush", - "lawngreen", "lemonchiffon", "lightblue", "lightcoral", "lightcyan", - "lightgoldenrodyellow", "lightgray", "lightgreen", "lightgrey", "lightpink", - "lightsalmon", "lightseagreen", "lightskyblue", "lightslategray", - "lightslategrey", "lightsteelblue", "lightyellow", "lime", "limegreen", "linen", - "magenta", "maroon", "mediumaquamarine", "mediumblue", "mediumorchid", - "mediumpurple", "mediumseagreen", "mediumslateblue", "mediumspringgreen", - "mediumturquoise", "mediumvioletred", "midnightblue", "mintcream", "mistyrose", - "moccasin", "navajowhite", "navy", "oldlace", "olive", "olivedrab", "orange", - "orangered", "orchid", "palegoldenrod", "palegreen", "paleturquoise", - "palevioletred", "papayawhip", "peachpuff", "peru", "pink", "plum", - "powderblue", "purple", "red", "rosybrown", "royalblue", "saddlebrown", - "salmon", "sandybrown", "seagreen", "seashell", "sienna", "silver", "skyblue", - "slateblue", "slategray", "slategrey", "snow", "springgreen", "steelblue", - "tan", "teal", "thistle", "tomato", "transparent", "turquoise", "violet", - "wheat", "white", "whitesmoke", "yellow", "yellowgreen", - } -) # fmt: skip +_COLOR_NAMES = frozenset(webcolors.names("css3")) | {"transparent"} class Outcome(tuple[bool, str]): diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 94656deaa..761813121 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -2,6 +2,7 @@ import itertools import pytest +import webcolors from tilia_core import rules, tla @@ -233,6 +234,12 @@ def test_a_text_not_in_nfc_is_kept(self): class TestColor: + def test_the_names_are_css_3s_and_transparent(self): + # The names Qt reads: CSS 3's, from webcolors, and Qt's "transparent". + for name in [*webcolors.names("css3"), "transparent"]: + assert rules.color(name), name + assert not rules.color("rebeccapurple") # CSS 4's, which Qt doesn't read + @pytest.mark.parametrize( "value", [ From 9e42e376671c22b6a246170e8224c1d1b6aefd5c Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:06:51 +0200 Subject: [PATCH 07/37] refactor(core): make the row id's format public ROW_ID_LENGTH and ROW_ID_CHARACTERS give the six letters and digits of a range row's id, as TiLiA's generate_row_id makes them, so that the importers make ids with the same format range_rows accepts. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 120d5653b..7b73754e1 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -63,8 +63,9 @@ _SEGMENT_TIMES = ("start", "end", "pre_start", "post_end") _UNSET_TIMES = ("pre_start", "post_end") # None: they follow the start and the end -_ROW_ID_LENGTH = 6 -_ROW_ID_CHARACTERS = frozenset(string.ascii_letters + string.digits) +# A range row's id, as TiLiA's `generate_row_id` makes it. +ROW_ID_LENGTH = 6 +ROW_ID_CHARACTERS = string.ascii_letters + string.digits # The colors Qt reads: #RGB, #RRGGBB, #AARRGGBB, #RRRGGGBBB and #RRRRGGGGBBBB, # and the names in `QColor.colorNames()`, which are CSS 3's (SVG 1.0's) and @@ -602,8 +603,8 @@ def _is_integer(value: Any) -> bool: def _is_row_id(value: Any) -> bool: return ( isinstance(value, str) - and len(value) == _ROW_ID_LENGTH - and all(c in _ROW_ID_CHARACTERS for c in value) + and len(value) == ROW_ID_LENGTH + and all(c in ROW_ID_CHARACTERS for c in value) ) From fa353af18ee2d6c681c1d69b4a65879b2bd82a0e Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:06:56 +0200 Subject: [PATCH 08/37] fix(core): an outcome and CantCheck copy and pickle whole Outcome is a tuple subclass whose __new__ takes the reason and the others apart, so copy, deepcopy and pickle rebuilt it from the pair alone, and CantCheck's pickled message came back as its kind. __getnewargs__ and __reduce__ give them what they need. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 7 +++++++ packages/tilia-core/tests/test_rules.py | 23 +++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 7b73754e1..94ac8c50d 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -94,6 +94,10 @@ def __new__(cls, ok: bool, reason: str = "", others: Iterable[str] = ()) -> Outc def __bool__(self) -> bool: return self[0] + def __getnewargs__(self) -> tuple[bool, str, tuple[str, ...]]: # type: ignore[override] + # What copy and pickle give __new__: a tuple's would be the pair alone. + return self[0], self[1], self.others + @property def ok(self) -> bool: return self[0] @@ -118,6 +122,9 @@ def __init__(self, kind: str) -> None: super().__init__(f"checking a {kind} needs: {INSTALL_HARMONY}") self.kind = kind + def __reduce__(self) -> tuple[type[CantCheck], tuple[str]]: + return type(self), (self.kind,) + # The private functions give the first rule they find broken, or None. `_first` # checks them in order, each only once the ones before it are kept: an outcome's diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 761813121..659c5be38 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -1,5 +1,7 @@ +import copy import importlib.util import itertools +import pickle import pytest import webcolors @@ -155,6 +157,27 @@ def test_a_broken_rule_unpacks_as_its_reason(self): assert (ok, reason) == (False, "Invalid row ID.") + @pytest.mark.parametrize( + "duplicate", [copy.copy, copy.deepcopy, lambda o: pickle.loads(pickle.dumps(o))] + ) + def test_an_outcome_copies_and_pickles_whole(self, duplicate): + outcome = rules.Outcome(False, "Invalid row ID.", ["a"]) + + copied = duplicate(outcome) + + assert copied == (False, "Invalid row ID.") + assert copied.others == ("a",) + assert not copied + assert duplicate(rules.OK) + + def test_cant_check_pickles_whole(self): + error = rules.CantCheck("harmony") + + copied = pickle.loads(pickle.dumps(error)) + + assert str(copied) == str(error) + assert copied.kind == "harmony" + def test_a_broken_rule_names_the_other_components(self): outcome = rules.Outcome(False, "Component would overlap", ["a", "b"]) From 53060faece93006e9c1951ee30bb117cd232c687 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:07:00 +0200 Subject: [PATCH 09/37] fix(core): an integer too large for a float isn't a valid time _is_number accepted any int, and the time checks then raised OverflowError when they converted it. It is now refused with the text for a value that isn't one. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 10 +++++++--- packages/tilia-core/tests/test_rules.py | 6 ++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 94ac8c50d..5afcbe361 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -597,10 +597,14 @@ def _media_time(seconds: float) -> str: def _is_number(value: Any) -> bool: - """A finite number, as JSON stores one: not a bool, nan or an infinity.""" - if isinstance(value, bool): + """A finite number, as JSON stores one: not a bool, nan, an infinity, or an + integer too large for a float.""" + if isinstance(value, bool) or not isinstance(value, (int, float)): + return False + try: + return math.isfinite(value) + except OverflowError: return False - return isinstance(value, int) or (isinstance(value, float) and math.isfinite(value)) def _is_integer(value: Any) -> bool: diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 659c5be38..cb69ec56b 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -224,6 +224,12 @@ def test_a_time_that_isnt_a_finite_number(self, value): f"{value} is not a valid time.", ) + def test_an_integer_too_large_for_a_float(self): + huge = 10**400 + + assert check("marker", time=huge) == (False, f"{huge} is not a valid time.") + assert check("hierarchy", end=huge) == (False, f"{huge} is not a valid end.") + def test_a_missing_time(self): assert rules.marker(MEDIA, [], {}) == (False, "None is not a valid time.") From 4919e4efa397a24f12a06b843867ecff3e890327 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:07:04 +0200 Subject: [PATCH 10/37] fix(core): a hierarchy crosses another whose level is written as a float _cross ignored a hierarchy whose level wasn't an int, so one with level 1.0 couldn't be crossed, although _same_place compares 1.0 with 1. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 2 +- packages/tilia-core/tests/test_rules.py | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 5afcbe361..855b5e855 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -497,7 +497,7 @@ def _cross(start: float, end: float, level: int, other: Mapping[str, Any]) -> bo other_start, other_end = other.get("start"), other.get("end") other_level = other.get("level") if not ( - _is_number(other_start) and _is_number(other_end) and _is_integer(other_level) + _is_number(other_start) and _is_number(other_end) and _is_number(other_level) ): return False if not (start < other_end and other_start < end): diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index cb69ec56b..741543acd 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -776,6 +776,13 @@ def test_every_hierarchy_it_crosses(self): assert outcome.others == tuple(h.id for h in crossed) + def test_crossing_one_whose_level_is_written_as_a_float(self): + other = hierarchy(0, 10, 1.0) + + outcome = check("hierarchy", [other], start=5, end=15, level=1) + + assert outcome == (False, "Component would overlap with existing component.") + def test_nesting_is_tilias_boundary_conflicts(self): # Every pair of hierarchies with times from 0 to 4 and levels from 1 to 8. spans = [ From 5ce4aab2876558398aba0194209a9c79ce1f4a3f Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:07:08 +0200 Subject: [PATCH 11/37] fix(core): round a time to tenths before counting its minutes A time just under a minute showed 60 seconds: 59.96 s was 00:60.0, as TiLiA's format_media_time shows it. It is rounded first, so it is 01:00.0 (TimeLineAnnotator/project-management#304 fixes TiLiA). Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 6 ++++-- packages/tilia-core/tests/test_rules.py | 12 ++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 855b5e855..74c0e97bb 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -587,11 +587,13 @@ def _qt_reads(text: str) -> bool: def _media_time(seconds: float) -> str: """A time as TiLiA's `format_media_time` shows it ("01:05.0", "1:02:05.0"), - with a minus sign before a negative one ("-00:01.0").""" + with a minus sign before a negative one ("-00:01.0"). It is rounded to tenths + before its minutes are counted, so 59.96 is "01:00.0", not "00:60.0".""" if seconds < 0: return "-" + _media_time(-seconds) + seconds = float(f"{seconds:.1f}") seconds_and_fraction = f"{seconds % 60:.1f}".zfill(4) - minutes = int(float(seconds) // 60) + minutes = int(seconds // 60) hours = f"{minutes // 60}:" if minutes >= 60 else "" return f"{hours}{str(minutes % 60).zfill(2)}:{seconds_and_fraction}" diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 741543acd..a145f1d14 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -413,6 +413,18 @@ def test_media_length_given_as_an_integer(self): "Time '01:01.0' is bigger than media time '01:00.0'", ) + @pytest.mark.parametrize( + "media_length, shown", + [(59.96, "01:00.0"), (3599.96, "1:00:00.0"), (59.94, "00:59.9")], + ) + def test_a_time_is_rounded_before_its_minutes_are_counted( + self, media_length, shown + ): + assert check("marker", time=7200.0, media_length=media_length) == ( + False, + f"Time '2:00:00.0' is bigger than media time '{shown}'", + ) + @pytest.mark.parametrize("kind", POINT_KINDS) @pytest.mark.parametrize("time", [0.0, 1.0, MEDIA]) def test_a_time_another_component_of_the_kind_has(self, kind, time): From d971e5d490ca870485f35d2540d825dd934b9d9e Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:07:12 +0200 Subject: [PATCH 12/37] fix(core): check a color last, so it can't hide another broken rule An importer keeps a row whose color isn't one, with the default color. A bad color was checked with the rules for every kind, before the others, so it hid any other broken rule of the same component, and the row was kept. The color is now checked last, for every kind and for a range timeline's rows. The test that TiLiA creates a component with any color goes: TiLiA's creation rules don't take a color. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 34 ++++++++++++++++----- packages/tilia-core/tests/test_rules.py | 31 ++++++++++++++++--- 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 74c0e97bb..23a3d6b58 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -128,7 +128,9 @@ def __reduce__(self) -> tuple[type[CantCheck], tuple[str]]: # The private functions give the first rule they find broken, or None. `_first` # checks them in order, each only once the ones before it are kept: an outcome's -# truth is whether it is kept, so `or` can't chain them. +# truth is whether it is kept, so `or` can't chain them. A color is checked last: +# an importer keeps a row whose color isn't one, with the default color, so a bad +# color mustn't hide another broken rule. Check = Callable[[], "Outcome | None"] @@ -150,14 +152,20 @@ def marker( media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] ) -> Outcome: """A marker.""" - return _first(lambda: _point("marker", media_length, others, attrs)) + return _first( + lambda: _point("marker", media_length, others, attrs), + lambda: _color_of(attrs), + ) def beat( media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] ) -> Outcome: """A beat. Its measure mark and beat unit follow the measure table's rules.""" - return _first(lambda: _point("beat", media_length, others, attrs)) + return _first( + lambda: _point("beat", media_length, others, attrs), + lambda: _color_of(attrs), + ) def pdf_marker( @@ -171,6 +179,7 @@ def pdf_marker( return _first( lambda: _point("pdf_marker", media_length, others, attrs), lambda: _page(attrs.get("page_number"), page_count), + lambda: _color_of(attrs), ) @@ -188,6 +197,7 @@ def harmony( lambda: _one_of(attrs, "level", _LEVELS), lambda: _one_of(attrs, "display_mode", HARMONY_DISPLAY_MODES), lambda: _one_of(attrs, "custom_text_font_type", FONT_TYPES), + lambda: _color_of(attrs), ) return _checked("harmony", broken, music21) @@ -203,6 +213,7 @@ def mode( lambda: _one_of(attrs, "accidental", HARMONY_ACCIDENTALS), lambda: _mode_type(music21, attrs) if music21 else None, lambda: _one_of(attrs, "level", _LEVELS), + lambda: _color_of(attrs), ) return _checked("mode", broken, music21) @@ -224,6 +235,7 @@ def hierarchy( lambda: _post_end(attrs.get("post_end"), end, media_length), lambda: _same_place(start, end, level, hierarchies), lambda: _crossed(start, end, level, hierarchies), + lambda: _color_of(attrs), ) @@ -248,6 +260,7 @@ def range_( lambda: _pre_start(attrs.get("pre_start"), start), lambda: _post_end(attrs.get("post_end"), end, media_length), lambda: _joined(attrs, others, rows), + lambda: _color_of(attrs), ) @@ -265,9 +278,10 @@ def range_rows(rows: Sequence[Mapping[str, Any]]) -> Outcome: name = row.get("name") if not isinstance(name, str): return _broken(not_valid("row_name", name)) - outcome = color(row.get("color")) - if not outcome.ok: - return outcome + for row in rows: + broken = _color_of(row) + if broken is not None: + return broken return OK @@ -326,7 +340,7 @@ def _point( def _every_kind(attrs: Mapping[str, Any], times: Iterable[str]) -> Outcome | None: - """Times are finite numbers, texts are text, and a color is one.""" + """Times are finite numbers and texts are text. The color is checked last.""" for name in times: value = attrs.get(name) if value is None and name in _UNSET_TIMES: @@ -336,9 +350,13 @@ def _every_kind(attrs: Mapping[str, Any], times: Iterable[str]) -> Outcome | Non for name in _TEXTS: if name in attrs and not isinstance(attrs[name], str): return _broken(not_valid(name, attrs[name])) + return None + + +def _color_of(attrs: Mapping[str, Any]) -> Outcome | None: if "color" in attrs: outcome = color(attrs["color"]) - if not outcome.ok: + if not outcome: return outcome return None diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index a145f1d14..5fe626e8c 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -316,10 +316,33 @@ def test_each_kind_with_a_color_checks_it(self, kind): "notacolor is not a valid color.", ) - def test_tilia_creates_a_component_with_any_color(self): - assert tilia_creates_point(1.0, color="notacolor") - assert tilia_creates_hierarchy(1.0, 2.0, 1, color="notacolor") - assert tilia_creates_range(1.0, 2.0, ROW, color="notacolor") + @pytest.mark.parametrize( + "kind, attrs, reason", + [ + ( + "marker", + {"time": 61.0}, + "Time '01:01.0' is bigger than media time '01:00.0'", + ), + ("hierarchy", {"level": 0}, "Level can't be below 1. Got '0'"), + ("range", {"row_id": "Zz99Zz"}, "Invalid row ID."), + ("marker", {"label": 3}, "3 is not a valid label."), + ], + ) + def test_a_bad_color_doesnt_hide_another_broken_rule(self, kind, attrs, reason): + # An importer keeps a row whose color isn't one, so the color is checked last. + assert check(kind, color="notacolor", **attrs) == (False, reason) + + def test_a_bad_row_color_doesnt_hide_another_broken_row(self): + rows = [ + {"id": ROW, "name": "Strings", "color": "notacolor"}, + {"id": ROW, "name": "Winds"}, + ] + + assert rules.range_rows(rows) == ( + False, + f"There is already a row with the id {ROW}.", + ) def test_colors_are_those_qt_reads(self): qt = pytest.importorskip("PySide6.QtGui", reason="compares with Qt") From ecf05aa618baf460314d9c60e464d66224fcca08 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:07:15 +0200 Subject: [PATCH 13/37] test(core): drop assertions TiLiA's creation rules can't fail The stand-ins for TiLiA's creation rules ignore what they don't take, so asserting that they accept a harmony's or a mode's values tested nothing. The comment above them now says which values TiLiA only checks in set_data. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/tests/test_rules.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 5fe626e8c..d27586e39 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -79,8 +79,9 @@ def find_spec_without_music21(name, *args, **kwargs): # TiLiA's creation rules, which its importers apply: `validate_creation` of its # point-like and segment-like components, and the range timeline's # `_validate_component_creation`. They say whether TiLiA creates a component, -# for the tests that show a rule the core adds; what they don't take, they -# don't check. +# for the tests that show a rule the core adds. They take only what TiLiA's +# creation rules look at: not a colour, a text or a harmony's values, which only +# `set_data` checks, so TiLiA's importers store any of them. def tilia_creates_point(time, existing_times=(), media_length=MEDIA, **_): @@ -549,7 +550,6 @@ def test_values_tilia_allows(self, attribute, values): ) def test_values_tilia_refuses_when_it_sets_them(self, attribute, value, reason): assert check("harmony", **{attribute: value}) == (False, reason) - assert tilia_creates_point(1.0, **{attribute: value}) def test_every_quality_music21_has(self, music21): for quality in music21.harmony.CHORD_TYPES: @@ -618,7 +618,6 @@ def test_values_tilia_allows(self, attribute, values): ) def test_values_tilia_refuses_when_it_sets_them(self, attribute, value, reason): assert check("mode", **{attribute: value}) == (False, reason) - assert tilia_creates_point(1.0, **{attribute: value}) def test_types_are_read_from_the_installed_music21(self, music21, monkeypatch): monkeypatch.setitem(music21.key.modeSharpsAlter, "made-up", 0) From 5709172e26ddbb6d701a3ea8ec43f1fe31e4e2e4 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:58:27 +0200 Subject: [PATCH 14/37] refactor(core): make range row ids with the rules' format new_row_id took its length and characters from a copy of its own, so the ids it made and the ids range_rows accepts could drift apart. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/importers/_commit.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/importers/_commit.py b/packages/tilia-core/src/tilia_core/importers/_commit.py index 09e76b1cf..aeeed7e8f 100644 --- a/packages/tilia-core/src/tilia_core/importers/_commit.py +++ b/packages/tilia-core/src/tilia_core/importers/_commit.py @@ -8,13 +8,10 @@ from __future__ import annotations import random -import string from collections.abc import Collection, Iterable from typing import Any -from tilia_core import tla - -_ROW_ID_CHARACTERS = string.ascii_letters + string.digits +from tilia_core import rules, tla def commit( @@ -42,7 +39,7 @@ def commit( def new_row_id(taken: Collection[str]) -> str: """A range row's id: six letters and digits, none of `taken`, as TiLiA makes them.""" while True: - row_id = "".join(random.choices(_ROW_ID_CHARACTERS, k=6)) + row_id = "".join(random.choices(rules.ROW_ID_CHARACTERS, k=rules.ROW_ID_LENGTH)) if row_id not in taken: return row_id From d65191fe87b90050c2a837adcc48366befb15529 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Wed, 14 Oct 2026 18:28:49 +0200 Subject: [PATCH 15/37] fix(core): a music21 that won't import can't check, rather than crash Checking a harmony or a mode with music21 installed but failing to import, for example against a numpy it can't use, raised that ImportError. It now raises CantCheck, as without the extra, with the ImportError as its cause. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 39 +++++++++++++-------- packages/tilia-core/tests/test_rules.py | 24 +++++++++++++ 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 23a3d6b58..ae4b2b45a 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -111,7 +111,8 @@ def reason(self) -> str: class CantCheck(Exception): - """The rules that need music21 can't be checked: the `harmony` extra is missing. + """The rules that need music21 can't be checked: the `harmony` extra is missing, + or music21 won't import (the error is the exception's cause). This isn't a broken rule: the component may well keep to them. """ @@ -187,7 +188,7 @@ def harmony( media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] ) -> Outcome: """A harmony. Without music21, raises `CantCheck` when no other rule is broken.""" - music21 = _music21() + music21, error = _music21() broken = _first_broken( lambda: _point("harmony", media_length, others, attrs), lambda: _one_of(attrs, "step", _STEPS), @@ -199,14 +200,14 @@ def harmony( lambda: _one_of(attrs, "custom_text_font_type", FONT_TYPES), lambda: _color_of(attrs), ) - return _checked("harmony", broken, music21) + return _checked("harmony", broken, music21, error) def mode( media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] ) -> Outcome: """A key. Without music21, raises `CantCheck` when no other rule is broken.""" - music21 = _music21() + music21, error = _music21() broken = _first_broken( lambda: _point("mode", media_length, others, attrs), lambda: _one_of(attrs, "step", _STEPS), @@ -215,7 +216,7 @@ def mode( lambda: _one_of(attrs, "level", _LEVELS), lambda: _color_of(attrs), ) - return _checked("mode", broken, music21) + return _checked("mode", broken, music21, error) def hierarchy( @@ -381,21 +382,29 @@ def _one_of( return _broken(not_valid(name, value)) -def _music21() -> ModuleType | None: - """music21, with what the rules read of it; None without the `harmony` extra.""" - if importlib.util.find_spec("music21") is None: - return None - import music21.harmony - import music21.key - - return music21 +def _music21() -> tuple[ModuleType | None, ImportError | None]: + """music21, with what the rules read of it; or None, without the `harmony` + extra, and with the error when music21 is installed but won't import.""" + try: + if importlib.util.find_spec("music21") is None: + return None, None + import music21.harmony + import music21.key + except ImportError as error: + return None, error + return music21, None -def _checked(kind: str, broken: Outcome | None, music21: ModuleType | None) -> Outcome: +def _checked( + kind: str, + broken: Outcome | None, + music21: ModuleType | None, + error: ImportError | None, +) -> Outcome: if broken is not None: return broken if music21 is None: - raise CantCheck(kind) + raise CantCheck(kind) from error return OK diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index d27586e39..91fecaa63 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -2,6 +2,7 @@ import importlib.util import itertools import pickle +import sys import pytest import webcolors @@ -76,6 +77,21 @@ def find_spec_without_music21(name, *args, **kwargs): monkeypatch.setattr(importlib.util, "find_spec", find_spec_without_music21) +@pytest.fixture +def broken_music21(monkeypatch): + # Installed, so found, but importing it fails, as with a numpy it can't use. + find_spec = importlib.util.find_spec + + def find_spec_with_music21(name, *args, **kwargs): + if name.partition(".")[0] == "music21": + return object() + return find_spec(name, *args, **kwargs) + + monkeypatch.setattr(importlib.util, "find_spec", find_spec_with_music21) + for name in ("music21", "music21.harmony", "music21.key"): + monkeypatch.setitem(sys.modules, name, None) + + # TiLiA's creation rules, which its importers apply: `validate_creation` of its # point-like and segment-like components, and the range timeline's # `_validate_component_creation`. They say whether TiLiA creates a component, @@ -634,6 +650,14 @@ def test_checking_fails_naming_the_install_command(self, kind, without_music21): assert INSTALL in str(raised.value) assert raised.value.command == INSTALL + @pytest.mark.parametrize("kind", ["harmony", "mode"]) + def test_a_music21_that_wont_import(self, kind, broken_music21): + with pytest.raises(rules.CantCheck) as raised: + getattr(rules, kind)(MEDIA, [], {"time": 1.0}) + + assert raised.value.command == INSTALL + assert isinstance(raised.value.__cause__, ImportError) + @pytest.mark.parametrize("kind", ["harmony", "mode"]) def test_the_other_rules_are_checked(self, kind, without_music21): attrs = {"time": 1.0, "level": 3, "quality": "maj", "type": "dorianx"} From dda95266dc39b8de5e86ab2e87440bf503d15221 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Wed, 14 Oct 2026 18:28:52 +0200 Subject: [PATCH 16/37] feat(core): declare the harmony extra CantCheck tells people to run pip install "tilia-core[harmony]", which installed nothing while the extra didn't exist. The extra brings music21, which only the harmony and mode checks, and later the harmony importer, import. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/pyproject.toml | 4 ++++ packages/tilia-core/tests/test_rules.py | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/packages/tilia-core/pyproject.toml b/packages/tilia-core/pyproject.toml index 2436a1760..f6969cb7c 100644 --- a/packages/tilia-core/pyproject.toml +++ b/packages/tilia-core/pyproject.toml @@ -17,6 +17,10 @@ authors = [ {name = "Anne Foo"} ] +[project.optional-dependencies] +# Checking harmonies and keys, and later importing them, reads music21. +harmony = ["music21>=9.1"] + [project.scripts] tilia = "tilia_core.commands:main" diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 91fecaa63..fcef344c7 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -3,6 +3,7 @@ import itertools import pickle import sys +from importlib.metadata import metadata import pytest import webcolors @@ -650,6 +651,9 @@ def test_checking_fails_naming_the_install_command(self, kind, without_music21): assert INSTALL in str(raised.value) assert raised.value.command == INSTALL + def test_the_command_installs_an_extra_the_core_has(self): + assert "harmony" in metadata("tilia-core").get_all("Provides-Extra", []) + @pytest.mark.parametrize("kind", ["harmony", "mode"]) def test_a_music21_that_wont_import(self, kind, broken_music21): with pytest.raises(rules.CantCheck) as raised: From 74ec1ce04991e19b0339ba7fa9b784aefdcb7e49 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:08:38 +0200 Subject: [PATCH 17/37] fix(core): an outcome's truth is a bool Outcome.__bool__ returned ok as it was given, so bool() raised TypeError for an outcome made with 1 or a numpy bool. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 2 +- packages/tilia-core/tests/test_rules.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index ae4b2b45a..017dbe056 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -92,7 +92,7 @@ def __new__(cls, ok: bool, reason: str = "", others: Iterable[str] = ()) -> Outc return outcome def __bool__(self) -> bool: - return self[0] + return bool(self[0]) def __getnewargs__(self) -> tuple[bool, str, tuple[str, ...]]: # type: ignore[override] # What copy and pickle give __new__: a tuple's would be the pair alone. diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index fcef344c7..3d2b6130a 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -165,6 +165,10 @@ def test_an_outcome_is_true_only_when_no_rule_is_broken(self): assert rules.OK assert not rules.Outcome(False, "Invalid row ID.") + def test_its_truth_is_a_bool_whatever_ok_was_given_as(self): + assert bool(rules.Outcome(1)) is True + assert bool(rules.Outcome(0, "Invalid row ID.")) is False + def test_if_reads_as_keeping_to_the_rules(self): assert rules.marker(60.0, [], {"time": 1.0}) assert not rules.marker(60.0, [], {"time": -1.0}) From 4049095db75d59dec2bf771215bb1b60a89fb0e9 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:08:44 +0200 Subject: [PATCH 18/37] fix(core): a hierarchy whose level is a huge integer can be crossed _cross took a level as a number only if a float could hold it, so a hierarchy with a level beyond that couldn't be crossed, although the level rule accepts any integer. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 4 +++- packages/tilia-core/tests/test_rules.py | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 017dbe056..c73141508 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -524,7 +524,9 @@ def _cross(start: float, end: float, level: int, other: Mapping[str, Any]) -> bo other_start, other_end = other.get("start"), other.get("end") other_level = other.get("level") if not ( - _is_number(other_start) and _is_number(other_end) and _is_number(other_level) + _is_number(other_start) + and _is_number(other_end) + and (_is_integer(other_level) or _is_number(other_level)) ): return False if not (start < other_end and other_start < end): diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 3d2b6130a..4569a07bc 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -842,6 +842,13 @@ def test_every_hierarchy_it_crosses(self): assert outcome.others == tuple(h.id for h in crossed) + def test_crossing_one_whose_level_is_a_huge_integer(self): + other = hierarchy(0, 10, 10**400) + + outcome = check("hierarchy", [other], start=5, end=15, level=1) + + assert outcome == (False, "Component would overlap with existing component.") + def test_crossing_one_whose_level_is_written_as_a_float(self): other = hierarchy(0, 10, 1.0) From 62f546a09a129a2294d0cb7407c0becd47452e84 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:09:39 +0200 Subject: [PATCH 19/37] fix(core): a bad color can't hide that a harmony couldn't be checked Without music21, a harmony or a key whose color isn't one gave the color's reason, not CantCheck, so an importer, which keeps a row with the default color, kept it with its quality or type unchecked. The color was checked inside the chain, before the decision on music21. One helper, _kept, now runs a kind's rules in order, then raises CantCheck when music21 is missing, then checks the color, so every kind gets that order from one place. A step that gives a kept outcome no longer ends the checks. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 92 +++++++++------------ packages/tilia-core/tests/test_rules.py | 13 +++ 2 files changed, 52 insertions(+), 53 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index c73141508..3d0c2b1a1 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -127,25 +127,34 @@ def __reduce__(self) -> tuple[type[CantCheck], tuple[str]]: return type(self), (self.kind,) -# The private functions give the first rule they find broken, or None. `_first` -# checks them in order, each only once the ones before it are kept: an outcome's -# truth is whether it is kept, so `or` can't chain them. A color is checked last: -# an importer keeps a row whose color isn't one, with the default color, so a bad -# color mustn't hide another broken rule. +# The private functions give the first rule they find broken, or None. `_kept` +# checks them in order, each only once the ones before it are kept. Then it raises +# `CantCheck` when the rules that need music21 couldn't be checked, and only then +# checks the color: an importer keeps a row whose color isn't one, with the +# default color, so a bad color mustn't hide another broken rule, nor that a rule +# couldn't be checked. Check = Callable[[], "Outcome | None"] -def _first_broken(*checks: Check) -> Outcome | None: +def _kept( + attrs: Mapping[str, Any], + *checks: Check, + missing: tuple[str, ImportError | None] | None = None, +) -> Outcome: + """The first rule `checks` find broken, else the color's outcome. + + `missing` is a harmony's or a key's kind, and the error if music21 is installed + but won't import, when music21 couldn't be imported to check them. + """ for check in checks: broken = check() - if broken is not None: + if broken is not None and not broken: return broken - return None - - -def _first(*checks: Check) -> Outcome: - broken = _first_broken(*checks) + if missing is not None: + kind, error = missing + raise CantCheck(kind) from error + broken = _color_of(attrs) return OK if broken is None else broken @@ -153,20 +162,14 @@ def marker( media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] ) -> Outcome: """A marker.""" - return _first( - lambda: _point("marker", media_length, others, attrs), - lambda: _color_of(attrs), - ) + return _kept(attrs, lambda: _point("marker", media_length, others, attrs)) def beat( media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] ) -> Outcome: """A beat. Its measure mark and beat unit follow the measure table's rules.""" - return _first( - lambda: _point("beat", media_length, others, attrs), - lambda: _color_of(attrs), - ) + return _kept(attrs, lambda: _point("beat", media_length, others, attrs)) def pdf_marker( @@ -177,10 +180,10 @@ def pdf_marker( page_count: int, ) -> Outcome: """A PDF marker, in a PDF of `page_count` pages: 0 for a PDF that can't be read.""" - return _first( + return _kept( + attrs, lambda: _point("pdf_marker", media_length, others, attrs), lambda: _page(attrs.get("page_number"), page_count), - lambda: _color_of(attrs), ) @@ -189,7 +192,8 @@ def harmony( ) -> Outcome: """A harmony. Without music21, raises `CantCheck` when no other rule is broken.""" music21, error = _music21() - broken = _first_broken( + return _kept( + attrs, lambda: _point("harmony", media_length, others, attrs), lambda: _one_of(attrs, "step", _STEPS), lambda: _one_of(attrs, "applied_to", _STEPS), @@ -198,9 +202,8 @@ def harmony( lambda: _one_of(attrs, "level", _LEVELS), lambda: _one_of(attrs, "display_mode", HARMONY_DISPLAY_MODES), lambda: _one_of(attrs, "custom_text_font_type", FONT_TYPES), - lambda: _color_of(attrs), + missing=None if music21 else ("harmony", error), ) - return _checked("harmony", broken, music21, error) def mode( @@ -208,15 +211,15 @@ def mode( ) -> Outcome: """A key. Without music21, raises `CantCheck` when no other rule is broken.""" music21, error = _music21() - broken = _first_broken( + return _kept( + attrs, lambda: _point("mode", media_length, others, attrs), lambda: _one_of(attrs, "step", _STEPS), lambda: _one_of(attrs, "accidental", HARMONY_ACCIDENTALS), lambda: _mode_type(music21, attrs) if music21 else None, lambda: _one_of(attrs, "level", _LEVELS), - lambda: _color_of(attrs), + missing=None if music21 else ("mode", error), ) - return _checked("mode", broken, music21, error) def hierarchy( @@ -224,19 +227,17 @@ def hierarchy( ) -> Outcome: """A hierarchy. Hierarchies nest: one that overlaps another lies inside it, at a lower level.""" - broken = _every_kind(attrs, _SEGMENT_TIMES) - if broken is not None: - return broken - start, end, level = attrs["start"], attrs["end"], attrs.get("level") + start, end, level = attrs.get("start"), attrs.get("end"), attrs.get("level") hierarchies = [other for other in others if other.kind == "hierarchy"] - return _first( + return _kept( + attrs, + lambda: _every_kind(attrs, _SEGMENT_TIMES), lambda: _hierarchy_times(start, end, media_length), lambda: _hierarchy_level(level), lambda: _pre_start(attrs.get("pre_start"), start), lambda: _post_end(attrs.get("post_end"), end, media_length), lambda: _same_place(start, end, level, hierarchies), lambda: _crossed(start, end, level, hierarchies), - lambda: _color_of(attrs), ) @@ -251,17 +252,15 @@ def range_( Ranges in a row may overlap. """ - broken = _every_kind(attrs, _SEGMENT_TIMES) - if broken is not None: - return broken - start, end = attrs["start"], attrs["end"] + start, end = attrs.get("start"), attrs.get("end") row_ids = [row.get("id") for row in rows] - return _first( + return _kept( + attrs, + lambda: _every_kind(attrs, _SEGMENT_TIMES), lambda: _range_place(attrs.get("row_id"), row_ids, start, end, media_length), lambda: _pre_start(attrs.get("pre_start"), start), lambda: _post_end(attrs.get("post_end"), end, media_length), lambda: _joined(attrs, others, rows), - lambda: _color_of(attrs), ) @@ -395,19 +394,6 @@ def _music21() -> tuple[ModuleType | None, ImportError | None]: return music21, None -def _checked( - kind: str, - broken: Outcome | None, - music21: ModuleType | None, - error: ImportError | None, -) -> Outcome: - if broken is not None: - return broken - if music21 is None: - raise CantCheck(kind) from error - return OK - - def _quality_and_inversion( music21: ModuleType, attrs: Mapping[str, Any] ) -> Outcome | None: diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 4569a07bc..3ed1a902b 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -174,6 +174,11 @@ def test_if_reads_as_keeping_to_the_rules(self): assert not rules.marker(60.0, [], {"time": -1.0}) assert not rules.hierarchy(60.0, [], {"start": 0.0, "end": 1.0, "level": 0}) + def test_a_check_that_keeps_doesnt_end_the_checks(self): + broken = rules.Outcome(False, "Invalid row ID.") + + assert rules._kept({}, lambda: rules.OK, lambda: broken) == broken + def test_a_broken_rule_unpacks_as_its_reason(self): ok, reason = rules.Outcome(False, "Invalid row ID.") @@ -666,6 +671,14 @@ def test_a_music21_that_wont_import(self, kind, broken_music21): assert raised.value.command == INSTALL assert isinstance(raised.value.__cause__, ImportError) + @pytest.mark.parametrize("kind", ["harmony", "mode"]) + def test_a_bad_color_doesnt_hide_that_it_cant_be_checked( + self, kind, without_music21 + ): + # An importer keeps a component whose only problem is its color. + with pytest.raises(rules.CantCheck): + getattr(rules, kind)(MEDIA, [], {"time": 1.0, "color": "notacolor"}) + @pytest.mark.parametrize("kind", ["harmony", "mode"]) def test_the_other_rules_are_checked(self, kind, without_music21): attrs = {"time": 1.0, "level": 3, "quality": "maj", "type": "dorianx"} From fa7a715bca5806130074d095e4ea67dc739d6719 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:11:08 +0200 Subject: [PATCH 20/37] fix(core): a music21 that fails to import in any way can't check Only ImportError was caught, so a music21 that raised something else on import, such as an AttributeError from a numpy it can't use, still crashed the check. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 9 +++++---- packages/tilia-core/tests/test_rules.py | 22 +++++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 3d0c2b1a1..4a030d506 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -140,7 +140,7 @@ def __reduce__(self) -> tuple[type[CantCheck], tuple[str]]: def _kept( attrs: Mapping[str, Any], *checks: Check, - missing: tuple[str, ImportError | None] | None = None, + missing: tuple[str, Exception | None] | None = None, ) -> Outcome: """The first rule `checks` find broken, else the color's outcome. @@ -381,15 +381,16 @@ def _one_of( return _broken(not_valid(name, value)) -def _music21() -> tuple[ModuleType | None, ImportError | None]: +def _music21() -> tuple[ModuleType | None, Exception | None]: """music21, with what the rules read of it; or None, without the `harmony` - extra, and with the error when music21 is installed but won't import.""" + extra, and with the error when music21 is installed but won't import, + whatever the error is.""" try: if importlib.util.find_spec("music21") is None: return None, None import music21.harmony import music21.key - except ImportError as error: + except Exception as error: return None, error return music21, None diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 3ed1a902b..44192d93b 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -93,6 +93,21 @@ def find_spec_with_music21(name, *args, **kwargs): monkeypatch.setitem(sys.modules, name, None) +@pytest.fixture +def music21_raising(tmp_path, monkeypatch): + # A music21 that is installed but raises something other than ImportError + # when imported, as with a numpy it can't use. + package = tmp_path / "music21" + package.mkdir() + (package / "__init__.py").write_text( + 'raise AttributeError("np.float_ was removed in the NumPy 2.0 release.")\n' + ) + for name in [name for name in sys.modules if name.partition(".")[0] == "music21"]: + monkeypatch.delitem(sys.modules, name) + monkeypatch.syspath_prepend(str(tmp_path)) + importlib.invalidate_caches() + + # TiLiA's creation rules, which its importers apply: `validate_creation` of its # point-like and segment-like components, and the range timeline's # `_validate_component_creation`. They say whether TiLiA creates a component, @@ -679,6 +694,13 @@ def test_a_bad_color_doesnt_hide_that_it_cant_be_checked( with pytest.raises(rules.CantCheck): getattr(rules, kind)(MEDIA, [], {"time": 1.0, "color": "notacolor"}) + @pytest.mark.parametrize("kind", ["harmony", "mode"]) + def test_a_music21_that_raises_something_else(self, kind, music21_raising): + with pytest.raises(rules.CantCheck) as raised: + getattr(rules, kind)(MEDIA, [], {"time": 1.0}) + + assert isinstance(raised.value.__cause__, AttributeError) + @pytest.mark.parametrize("kind", ["harmony", "mode"]) def test_the_other_rules_are_checked(self, kind, without_music21): attrs = {"time": 1.0, "level": 3, "quality": "maj", "type": "dorianx"} From 1a3527020383756644770b4a7412fee326872971 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:11:17 +0200 Subject: [PATCH 21/37] fix(core): CantCheck says when music21 is installed but won't import It told people to install the harmony extra even when it was installed and music21 failed to import, so a printed report gave no way to a fix. It now says that music21 won't import, and why. The text for a missing extra is unchanged. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 18 +++++++++++++----- packages/tilia-core/tests/test_rules.py | 5 +++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 4a030d506..31243fcd9 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -119,12 +119,20 @@ class CantCheck(Exception): command = INSTALL_HARMONY - def __init__(self, kind: str) -> None: - super().__init__(f"checking a {kind} needs: {INSTALL_HARMONY}") + def __init__(self, kind: str, error: Exception | None = None) -> None: + if error is None: + message = f"checking a {kind} needs: {INSTALL_HARMONY}" + else: + message = ( + f"checking a {kind} needs music21, which is installed but can't" + f" be imported: {type(error).__name__}: {error}" + ) + super().__init__(message) self.kind = kind + self.error = error - def __reduce__(self) -> tuple[type[CantCheck], tuple[str]]: - return type(self), (self.kind,) + def __reduce__(self) -> tuple[type[CantCheck], tuple[str, Exception | None]]: + return type(self), (self.kind, self.error) # The private functions give the first rule they find broken, or None. `_kept` @@ -153,7 +161,7 @@ def _kept( return broken if missing is not None: kind, error = missing - raise CantCheck(kind) from error + raise CantCheck(kind, error) from error broken = _color_of(attrs) return OK if broken is None else broken diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 44192d93b..8485c1bf4 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -700,6 +700,11 @@ def test_a_music21_that_raises_something_else(self, kind, music21_raising): getattr(rules, kind)(MEDIA, [], {"time": 1.0}) assert isinstance(raised.value.__cause__, AttributeError) + assert str(raised.value) == ( + f"checking a {kind} needs music21, which is installed but can't be" + " imported: AttributeError: np.float_ was removed in the NumPy 2.0" + " release." + ) @pytest.mark.parametrize("kind", ["harmony", "mode"]) def test_the_other_rules_are_checked(self, kind, without_music21): From 735d4945b501f40051780c5ec17f463681b26cd4 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:11:25 +0200 Subject: [PATCH 22/37] perf(core): don't import a broken music21 again for every harmony Python keeps no module whose import failed, so each harmony or key checked tried the failing import again. The failure is kept for the session. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 9 +++++++++ packages/tilia-core/tests/test_rules.py | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 31243fcd9..eaad8897b 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -389,16 +389,25 @@ def _one_of( return _broken(not_valid(name, value)) +# A music21 that wouldn't import, kept so that it isn't imported again for every +# harmony: Python doesn't keep a module whose import failed. +_music21_failure: Exception | None = None + + def _music21() -> tuple[ModuleType | None, Exception | None]: """music21, with what the rules read of it; or None, without the `harmony` extra, and with the error when music21 is installed but won't import, whatever the error is.""" + global _music21_failure + if _music21_failure is not None: + return None, _music21_failure try: if importlib.util.find_spec("music21") is None: return None, None import music21.harmony import music21.key except Exception as error: + _music21_failure = error return None, error return music21, None diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 8485c1bf4..cfc9fcb24 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -91,6 +91,7 @@ def find_spec_with_music21(name, *args, **kwargs): monkeypatch.setattr(importlib.util, "find_spec", find_spec_with_music21) for name in ("music21", "music21.harmony", "music21.key"): monkeypatch.setitem(sys.modules, name, None) + monkeypatch.setattr(rules, "_music21_failure", None, raising=False) @pytest.fixture @@ -106,6 +107,7 @@ def music21_raising(tmp_path, monkeypatch): monkeypatch.delitem(sys.modules, name) monkeypatch.syspath_prepend(str(tmp_path)) importlib.invalidate_caches() + monkeypatch.setattr(rules, "_music21_failure", None, raising=False) # TiLiA's creation rules, which its importers apply: `validate_creation` of its @@ -706,6 +708,23 @@ def test_a_music21_that_raises_something_else(self, kind, music21_raising): " release." ) + def test_a_failed_import_isnt_tried_again(self, music21_raising, monkeypatch): + find_spec = importlib.util.find_spec + looked_up = [] + + def counting(name, *args, **kwargs): + if name == "music21": + looked_up.append(name) + return find_spec(name, *args, **kwargs) + + monkeypatch.setattr(importlib.util, "find_spec", counting) + + for kind in ("harmony", "mode", "harmony"): + with pytest.raises(rules.CantCheck): + getattr(rules, kind)(MEDIA, [], {"time": 1.0}) + + assert len(looked_up) == 1 + @pytest.mark.parametrize("kind", ["harmony", "mode"]) def test_the_other_rules_are_checked(self, kind, without_music21): attrs = {"time": 1.0, "level": 3, "quality": "maj", "type": "dorianx"} From f1f3de0ec806fa0807f52b48a780dfcd2511a200 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:38:49 +0200 Subject: [PATCH 23/37] revert(core): don't keep a failed music21 import for the session This reverts 735d4945. Keeping the failure for good meant that installing a missing dependency in a notebook didn't help until a restart, kept the failed import's frames alive, and made the tests depend on their order; and it left music21's absence looked up every time anyway. The loops it was for go through check_timeline, which can look music21 up once per call (TimeLineAnnotator/project-management#303). Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 9 --------- packages/tilia-core/tests/test_rules.py | 19 ------------------- 2 files changed, 28 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index eaad8897b..31243fcd9 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -389,25 +389,16 @@ def _one_of( return _broken(not_valid(name, value)) -# A music21 that wouldn't import, kept so that it isn't imported again for every -# harmony: Python doesn't keep a module whose import failed. -_music21_failure: Exception | None = None - - def _music21() -> tuple[ModuleType | None, Exception | None]: """music21, with what the rules read of it; or None, without the `harmony` extra, and with the error when music21 is installed but won't import, whatever the error is.""" - global _music21_failure - if _music21_failure is not None: - return None, _music21_failure try: if importlib.util.find_spec("music21") is None: return None, None import music21.harmony import music21.key except Exception as error: - _music21_failure = error return None, error return music21, None diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index cfc9fcb24..8485c1bf4 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -91,7 +91,6 @@ def find_spec_with_music21(name, *args, **kwargs): monkeypatch.setattr(importlib.util, "find_spec", find_spec_with_music21) for name in ("music21", "music21.harmony", "music21.key"): monkeypatch.setitem(sys.modules, name, None) - monkeypatch.setattr(rules, "_music21_failure", None, raising=False) @pytest.fixture @@ -107,7 +106,6 @@ def music21_raising(tmp_path, monkeypatch): monkeypatch.delitem(sys.modules, name) monkeypatch.syspath_prepend(str(tmp_path)) importlib.invalidate_caches() - monkeypatch.setattr(rules, "_music21_failure", None, raising=False) # TiLiA's creation rules, which its importers apply: `validate_creation` of its @@ -708,23 +706,6 @@ def test_a_music21_that_raises_something_else(self, kind, music21_raising): " release." ) - def test_a_failed_import_isnt_tried_again(self, music21_raising, monkeypatch): - find_spec = importlib.util.find_spec - looked_up = [] - - def counting(name, *args, **kwargs): - if name == "music21": - looked_up.append(name) - return find_spec(name, *args, **kwargs) - - monkeypatch.setattr(importlib.util, "find_spec", counting) - - for kind in ("harmony", "mode", "harmony"): - with pytest.raises(rules.CantCheck): - getattr(rules, kind)(MEDIA, [], {"time": 1.0}) - - assert len(looked_up) == 1 - @pytest.mark.parametrize("kind", ["harmony", "mode"]) def test_the_other_rules_are_checked(self, kind, without_music21): attrs = {"time": 1.0, "level": 3, "quality": "maj", "type": "dorianx"} From 627c66ad0b76eb7a641702decdf81f08255788c9 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:40:03 +0200 Subject: [PATCH 24/37] fix(core): CantCheck gives the reason and the install command When music21 was found but wouldn't import, the message said it was installed and gave no install command. But a leftover music21 folder, or a missing dependency, also gives an ImportError, and installing the extra fixes both. The message now gives the reason and the command, and doesn't claim music21 is installed. The text for a missing extra is unchanged. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 4 ++-- packages/tilia-core/tests/test_rules.py | 12 ++++++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 31243fcd9..045568b8d 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -124,8 +124,8 @@ def __init__(self, kind: str, error: Exception | None = None) -> None: message = f"checking a {kind} needs: {INSTALL_HARMONY}" else: message = ( - f"checking a {kind} needs music21, which is installed but can't" - f" be imported: {type(error).__name__}: {error}" + f"checking a {kind} needs music21, which can't be imported" + f" ({type(error).__name__}: {error}): {INSTALL_HARMONY}" ) super().__init__(message) self.kind = kind diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 8485c1bf4..bb6d5e39e 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -683,8 +683,12 @@ def test_a_music21_that_wont_import(self, kind, broken_music21): with pytest.raises(rules.CantCheck) as raised: getattr(rules, kind)(MEDIA, [], {"time": 1.0}) - assert raised.value.command == INSTALL assert isinstance(raised.value.__cause__, ImportError) + assert str(raised.value) == ( + f"checking a {kind} needs music21, which can't be imported" + " (ModuleNotFoundError: import of music21.harmony halted; None in" + f" sys.modules): {INSTALL}" + ) @pytest.mark.parametrize("kind", ["harmony", "mode"]) def test_a_bad_color_doesnt_hide_that_it_cant_be_checked( @@ -701,9 +705,9 @@ def test_a_music21_that_raises_something_else(self, kind, music21_raising): assert isinstance(raised.value.__cause__, AttributeError) assert str(raised.value) == ( - f"checking a {kind} needs music21, which is installed but can't be" - " imported: AttributeError: np.float_ was removed in the NumPy 2.0" - " release." + f"checking a {kind} needs music21, which can't be imported" + " (AttributeError: np.float_ was removed in the NumPy 2.0 release.):" + f" {INSTALL}" ) @pytest.mark.parametrize("kind", ["harmony", "mode"]) From 28ba92ff109de79b86ff8433a5c3c851789069f9 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:40:12 +0200 Subject: [PATCH 25/37] fix(core): a CantCheck copies and pickles whatever its error Since CantCheck carried the import error, copying or pickling one whose error class needs arguments to be built raised TypeError. The error isn't carried any more: its text is in the message, which is. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 6 ++++-- packages/tilia-core/tests/test_rules.py | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 045568b8d..3c7b3e466 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -131,8 +131,10 @@ def __init__(self, kind: str, error: Exception | None = None) -> None: self.kind = kind self.error = error - def __reduce__(self) -> tuple[type[CantCheck], tuple[str, Exception | None]]: - return type(self), (self.kind, self.error) + def __reduce__(self) -> tuple[Any, ...]: + # The error isn't carried, since it may not copy or pickle: its text is in + # the message, which is. + return type(self), (self.kind,), {"args": self.args, "error": None} # The private functions give the first rule they find broken, or None. `_kept` diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index bb6d5e39e..b32294649 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -220,6 +220,22 @@ def test_cant_check_pickles_whole(self): assert str(copied) == str(error) assert copied.kind == "harmony" + def test_cant_check_copies_and_pickles_whatever_its_error(self): + class Picky(Exception): + def __init__(self, a, b): + super().__init__(f"{a} {b}") + + error = rules.CantCheck("harmony", Picky(1, 2)) + + for duplicate in ( + copy.copy, + copy.deepcopy, + lambda e: pickle.loads(pickle.dumps(e)), + ): + copied = duplicate(error) + assert str(copied) == str(error) + assert copied.kind == "harmony" + def test_a_broken_rule_names_the_other_components(self): outcome = rules.Outcome(False, "Component would overlap", ["a", "b"]) From cc59c0999d63a0d7c64e81ec6998d9149b2de395 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:40:19 +0200 Subject: [PATCH 26/37] fix(core): an outcome's ok is a bool however it's read Only __bool__ converted it, so .ok and unpacking still gave 1, or a numpy bool, when the outcome was made with one. It is converted once, when the outcome is made. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 4 ++-- packages/tilia-core/tests/test_rules.py | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 3c7b3e466..8395d607e 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -87,12 +87,12 @@ class Outcome(tuple[bool, str]): others: tuple[str, ...] def __new__(cls, ok: bool, reason: str = "", others: Iterable[str] = ()) -> Outcome: - outcome = super().__new__(cls, (ok, reason)) + outcome = super().__new__(cls, (bool(ok), reason)) outcome.others = tuple(others) return outcome def __bool__(self) -> bool: - return bool(self[0]) + return self[0] def __getnewargs__(self) -> tuple[bool, str, tuple[str, ...]]: # type: ignore[override] # What copy and pickle give __new__: a tuple's would be the pair alone. diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index b32294649..fe961d458 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -184,6 +184,11 @@ def test_its_truth_is_a_bool_whatever_ok_was_given_as(self): assert bool(rules.Outcome(1)) is True assert bool(rules.Outcome(0, "Invalid row ID.")) is False + def test_ok_is_a_bool_however_its_read(self): + assert rules.Outcome(1).ok is True + ok, _ = rules.Outcome(0, "Invalid row ID.") + assert ok is False + def test_if_reads_as_keeping_to_the_rules(self): assert rules.marker(60.0, [], {"time": 1.0}) assert not rules.marker(60.0, [], {"time": -1.0}) From fe558e86c57d65061d0499909a5048ae0e59a9bc Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:40:27 +0200 Subject: [PATCH 27/37] fix(core): compare a hierarchy's start and end of any integer size A hierarchy whose start or end was an integer too large for a float was skipped when looking for crossings, as its level was until 4049095d. Its own check reports the value; the others are still compared with it. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 12 +++++++++--- packages/tilia-core/tests/test_rules.py | 7 +++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 8395d607e..6cb380d65 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -521,9 +521,9 @@ def _cross(start: float, end: float, level: int, other: Mapping[str, Any]) -> bo other_start, other_end = other.get("start"), other.get("end") other_level = other.get("level") if not ( - _is_number(other_start) - and _is_number(other_end) - and (_is_integer(other_level) or _is_number(other_level)) + _is_comparable(other_start) + and _is_comparable(other_end) + and _is_comparable(other_level) ): return False if not (start < other_end and other_start < end): @@ -635,6 +635,12 @@ def _is_number(value: Any) -> bool: return False +def _is_comparable(value: Any) -> bool: + """A number another can be compared with: an integer of any size, or a + finite float. A component that breaks a rule with one is still compared.""" + return _is_integer(value) or _is_number(value) + + def _is_integer(value: Any) -> bool: return isinstance(value, int) and not isinstance(value, bool) diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index fe961d458..ab6b33c40 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -907,6 +907,13 @@ def test_every_hierarchy_it_crosses(self): assert outcome.others == tuple(h.id for h in crossed) + def test_crossing_one_whose_start_is_a_huge_integer(self): + other = hierarchy(-(10**400), 10, 1) + + outcome = check("hierarchy", [other], start=5, end=15, level=1) + + assert outcome == (False, "Component would overlap with existing component.") + def test_crossing_one_whose_level_is_a_huge_integer(self): other = hierarchy(0, 10, 10**400) From 086b657235d841b2f976d158d85480234c103f75 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:40:33 +0200 Subject: [PATCH 28/37] refactor(core): give _kept a ready CantCheck harmony and mode built a (kind, error) pair for _kept to unpack into a CantCheck; they now build the CantCheck. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 6cb380d65..fdca6cd58 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -150,20 +150,16 @@ def __reduce__(self) -> tuple[Any, ...]: def _kept( attrs: Mapping[str, Any], *checks: Check, - missing: tuple[str, Exception | None] | None = None, + cant_check: CantCheck | None = None, ) -> Outcome: - """The first rule `checks` find broken, else the color's outcome. - - `missing` is a harmony's or a key's kind, and the error if music21 is installed - but won't import, when music21 couldn't be imported to check them. - """ + """The first rule `checks` find broken, else `cant_check` raised, when the + rules that need music21 couldn't be checked, else the color's outcome.""" for check in checks: broken = check() if broken is not None and not broken: return broken - if missing is not None: - kind, error = missing - raise CantCheck(kind, error) from error + if cant_check is not None: + raise cant_check from cant_check.error broken = _color_of(attrs) return OK if broken is None else broken @@ -212,7 +208,7 @@ def harmony( lambda: _one_of(attrs, "level", _LEVELS), lambda: _one_of(attrs, "display_mode", HARMONY_DISPLAY_MODES), lambda: _one_of(attrs, "custom_text_font_type", FONT_TYPES), - missing=None if music21 else ("harmony", error), + cant_check=None if music21 else CantCheck("harmony", error), ) @@ -228,7 +224,7 @@ def mode( lambda: _one_of(attrs, "accidental", HARMONY_ACCIDENTALS), lambda: _mode_type(music21, attrs) if music21 else None, lambda: _one_of(attrs, "level", _LEVELS), - missing=None if music21 else ("mode", error), + cant_check=None if music21 else CantCheck("mode", error), ) From 60abddd48791e9c571ad6dd0b56e67cb8989b79d Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:40:38 +0200 Subject: [PATCH 29/37] docs(core): say that CantCheck comes before the color The docstrings said CantCheck is raised when no other rule is broken, but the color is checked after it, so that a bad color can't hide that a rule couldn't be checked. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index fdca6cd58..2e28cb8a8 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -17,8 +17,9 @@ A harmony's quality and inversion and a mode's type are checked against the installed music21, as TiLiA checks them, so checking a harmony or a mode needs the `harmony` extra. Without it, the other rules are checked, and when none is -broken, `CantCheck` is raised. music21 is imported only when a harmony or a mode -is checked. +broken, `CantCheck` is raised, before the color is checked: a bad color mustn't +hide that a rule couldn't be checked. music21 is imported only when a harmony or +a mode is checked. """ from __future__ import annotations @@ -196,7 +197,8 @@ def pdf_marker( def harmony( media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] ) -> Outcome: - """A harmony. Without music21, raises `CantCheck` when no other rule is broken.""" + """A harmony. Without music21, raises `CantCheck` when no rule but the color is + broken: the color is checked after it.""" music21, error = _music21() return _kept( attrs, @@ -215,7 +217,8 @@ def harmony( def mode( media_length: float, others: Iterable[tla.Component], attrs: Mapping[str, Any] ) -> Outcome: - """A key. Without music21, raises `CantCheck` when no other rule is broken.""" + """A key. Without music21, raises `CantCheck` when no rule but the color is + broken: the color is checked after it.""" music21, error = _music21() return _kept( attrs, From 52ed2fdf8fa2384817cfefc0b056ace6b075b73b Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:40:44 +0200 Subject: [PATCH 30/37] test(core): drop the test of _kept's guard No check a public function makes gives a kept outcome, so the guard it tested can't be reached from outside, and the test was tied to a private helper. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/tests/test_rules.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index ab6b33c40..78dfc6c89 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -194,11 +194,6 @@ def test_if_reads_as_keeping_to_the_rules(self): assert not rules.marker(60.0, [], {"time": -1.0}) assert not rules.hierarchy(60.0, [], {"start": 0.0, "end": 1.0, "level": 0}) - def test_a_check_that_keeps_doesnt_end_the_checks(self): - broken = rules.Outcome(False, "Invalid row ID.") - - assert rules._kept({}, lambda: rules.OK, lambda: broken) == broken - def test_a_broken_rule_unpacks_as_its_reason(self): ok, reason = rules.Outcome(False, "Invalid row ID.") From 494518cbf6ee6519f40bc61f28dec4d1b136e25b Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:16:45 +0200 Subject: [PATCH 31/37] fix(core): give a reason for an integer too long to show Python won't turn an integer of more than 4,300 digits into text, so the reason for one, such as a time of 10**5000 from a bulk edit, raised ValueError instead. Reasons show such a value as "an integer too long to show". Files can't hold one: json refuses it. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 13 +++++++++++-- packages/tilia-core/tests/test_rules.py | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 2e28cb8a8..0446fdd6f 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -313,7 +313,7 @@ def not_valid(attribute: str, value: Any) -> str: The rules give it for a value they refuse as not being one, such as a time of nan, and the importers for a value they can't read, so the two read alike. """ - return f"{value} is not a valid {attribute.replace('_', ' ')}." + return f"{_shown(value)} is not a valid {attribute.replace('_', ' ')}." def _point( @@ -453,7 +453,7 @@ def _hierarchy_level(level: Any) -> Outcome | None: if not _is_integer(level): return _broken(not_valid("level", level)) if level < 1: - return _broken(f"Level can't be below 1. Got '{level}'") + return _broken(f"Level can't be below 1. Got '{_shown(level)}'") return None @@ -652,5 +652,14 @@ def _is_row_id(value: Any) -> bool: ) +def _shown(value: Any) -> str: + """`value` as a reason shows it. An integer Python won't turn into text, one + of more than 4,300 digits unless set otherwise, is named instead.""" + try: + return f"{value}" + except ValueError: + return "an integer too long to show" + + def _broken(reason: str) -> Outcome: return Outcome(False, reason) diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 78dfc6c89..693255daf 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -288,6 +288,25 @@ def test_an_integer_too_large_for_a_float(self): assert check("marker", time=huge) == (False, f"{huge} is not a valid time.") assert check("hierarchy", end=huge) == (False, f"{huge} is not a valid end.") + def test_an_integer_too_long_to_show(self): + if not hasattr(sys, "set_int_max_str_digits"): + pytest.skip("this Python shows integers of any length") + limit = sys.get_int_max_str_digits() + sys.set_int_max_str_digits(4300) + try: + huge = 10**5000 + + assert check("marker", time=huge) == ( + False, + "an integer too long to show is not a valid time.", + ) + assert check("hierarchy", level=-huge) == ( + False, + "Level can't be below 1. Got 'an integer too long to show'", + ) + finally: + sys.set_int_max_str_digits(limit) + def test_a_missing_time(self): assert rules.marker(MEDIA, [], {}) == (False, "None is not a valid time.") From 3cd8e4fabaf095aa298c89d82c7948983bd33960 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:17:18 +0200 Subject: [PATCH 32/37] fix(core): hierarchy boundaries a few ulps apart meet, as on dev TiLiA's get_boundary_conflicts compares boundaries with a 1e-9 tolerance since dev's 2ceec157, because files saved before then hold boundaries a few ulps apart where they should meet. The core compared them exactly, so such a file's hierarchies crossed, and an import skipped rows TiLiA keeps. The test's copy of get_boundary_conflicts is dev's now, and a sweep adds boundaries 1e-13 apart. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 30 ++++++-- packages/tilia-core/tests/test_rules.py | 76 +++++++++++++++++++-- 2 files changed, 97 insertions(+), 9 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 0446fdd6f..9c951ada4 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -51,6 +51,11 @@ {"Italian", "French", "German", "Neapolitan", "Tristan", "power"} ) +# Hierarchy boundaries less than this far apart count as the same, as TiLiA's +# `get_boundary_conflicts` compares them since `dev`'s 2ceec157: files TiLiA saved +# before then hold boundaries a few ulps apart where they should meet. +_TIME_ABS_TOL = 1e-9 + # The names TiLiA's reasons give the point kinds (their `frontend_name`). _POINT_NAMES = { "marker": "marker", @@ -515,7 +520,8 @@ def _crossed( def _cross(start: float, end: float, level: int, other: Mapping[str, Any]) -> bool: """Whether two hierarchies overlap, and neither lies inside the other, lower. - This is when TiLiA's `get_boundary_conflicts` finds a conflict between them. + This is when TiLiA's `get_boundary_conflicts` finds a conflict between them, as + `dev` has it since 2ceec157: times less than 1e-9 apart count as the same. """ other_start, other_end = other.get("start"), other.get("end") other_level = other.get("level") @@ -525,13 +531,29 @@ def _cross(start: float, end: float, level: int, other: Mapping[str, Any]) -> bo and _is_comparable(other_level) ): return False - if not (start < other_end and other_start < end): + if not (_lt(start, other_end) and _lt(other_start, end)): return False - inside = other_start <= start and end <= other_end and level < other_level - around = start <= other_start and other_end <= end and other_level < level + inside = _le(other_start, start) and _le(end, other_end) and level < other_level + around = _le(start, other_start) and _le(other_end, end) and other_level < level return not (inside or around) +def _times_close(a: float, b: float) -> bool: + """Whether two times count as the same, as TiLiA's `_times_close` says.""" + try: + return math.isclose(a, b, abs_tol=_TIME_ABS_TOL) + except OverflowError: # an integer too large for a float is close only to itself + return a == b + + +def _le(a: float, b: float) -> bool: + return a < b or _times_close(a, b) + + +def _lt(a: float, b: float) -> bool: + return a < b and not _times_close(a, b) + + def _range_place( row_id: Any, row_ids: Sequence[Any], start: float, end: float, media_length: float ) -> Outcome | None: diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 693255daf..5d1484004 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -1,7 +1,9 @@ import copy import importlib.util import itertools +import math import pickle +import random import sys from importlib.metadata import metadata @@ -150,23 +152,49 @@ def tilia_creates_range( return True +# `dev`'s comparisons of hierarchy boundaries since 2ceec157: times less than +# 1e-9 apart count as the same. + + +def _times_close(a, b): + return math.isclose(a, b, abs_tol=1e-9) + + +def _le(a, b): + return a < b or _times_close(a, b) + + +def _lt(a, b): + return a < b and not _times_close(a, b) + + def tilia_boundary_conflicts(hierarchies): """`HierarchyTLComponentManager.get_boundary_conflicts`, on (start, end, level)s.""" + + def starts_or_ends_strictly_inside(outer, inner): + return (_lt(outer[0], inner[0]) and _lt(inner[0], outer[1])) or ( + _lt(outer[0], inner[1]) and _lt(inner[1], outer[1]) + ) + conflicts = [] for hrc1, hrc2 in itertools.combinations(hierarchies, 2): start1, end1, level1 = hrc1 start2, end2, level2 = hrc2 - if start1 < start2 < end1 or start1 < end2 < end1: + if starts_or_ends_strictly_inside(hrc1, hrc2): if level2 >= level1: conflicts.append((hrc1, hrc2)) - elif not start1 > start2 and end1 < end2: + elif not _lt(start2, start1) and _lt(end1, end2): conflicts.append((hrc1, hrc2)) - if start2 < start1 < end2 or start2 < end1 < end2: + if starts_or_ends_strictly_inside(hrc2, hrc1): if level1 >= level2: conflicts.append((hrc2, hrc1)) - elif not start2 > start1 and end2 < end1: + elif not _lt(start1, start2) and _lt(end2, end1): conflicts.append((hrc2, hrc1)) - if start1 == start2 and end1 == end2 and level1 == level2: + if ( + _times_close(start1, start2) + and _times_close(end1, end2) + and level1 == level2 + ): conflicts.append((hrc1, hrc2)) return conflicts @@ -958,6 +986,44 @@ def test_nesting_is_tilias_boundary_conflicts(self): conflicts = tilia_boundary_conflicts([existing, candidate]) assert outcome.ok is not bool(conflicts), (existing, candidate) + def test_boundaries_a_few_ulps_apart_meet(self): + # As in files TiLiA saved before it rescaled subtrees in one step. + others = [hierarchy(0.0, 1.0, 1)] + + assert check("hierarchy", others, start=1.0 - 1e-13, end=2.0, level=1) == OK + + def test_nesting_with_float_noise_is_tilias_boundary_conflicts(self): + # Every pair of spans from 1 to 4 s at levels 1 to 3, with each boundary + # moved by up to 1e-13. + noise = random.Random(2026) + spans = [ + (start, end, level) + for start, end in itertools.combinations(range(1, 5), 2) + for level in (1, 2, 3) + ] + for existing, candidate in itertools.product(spans, repeat=2): + for _ in range(4): + moved = [ + ( + start + noise.choice((-1e-13, 0, 1e-13)), + end + noise.choice((-1e-13, 0, 1e-13)), + level, + ) + for start, end, level in (existing, candidate) + ] + start, end, level = moved[1] + + outcome = check( + "hierarchy", + [hierarchy(*moved[0])], + start=start, + end=end, + level=level, + ) + + conflicts = tilia_boundary_conflicts(moved) + assert outcome.ok is not bool(conflicts), moved + def test_other_kinds_arent_compared(self): others = [component("marker", time=1.0), range_in(ROW, 0.0, 5.0)] From 01eb2c75be5f290521fa8ac00f769b78fe8ca5f6 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:17:31 +0200 Subject: [PATCH 33/37] feat(core): accept TiLiA's added-tone qualities, as on dev dev's 43a61224 and 28539120 add 13 qualities music21 has no chord type for, each a chord type plus an added tone (dominant-seventh-flat-ninth and so on). TiLiA writes them, and its symbol reading, which the harmony importer moves into the core, makes them. ADDED_TONE_QUALITIES and split_added_tone come into the core, so the list is kept once, and a quality's inversions are counted from its base, as TiLiA counts them. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 36 +++++++++++++++++++-- packages/tilia-core/tests/test_rules.py | 31 ++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 9c951ada4..5f647e46f 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -46,6 +46,25 @@ _STEPS = range(7) _LEVELS = (1, 2) _NOT_MODE_TYPES = ("ionian", "aeolian") +# Qualities music21 has no chord type for, each a chord type it has plus one added +# tone, spelled as in a chord symbol, from the root: TiLiA's, as `dev` has them +# since 43a61224 and 28539120. TiLiA takes them from here once it checks with +# the rules. +ADDED_TONE_QUALITIES: dict[str, tuple[str, str]] = { + "dominant-seventh-flat-ninth": ("dominant-seventh", "b9"), + "dominant-seventh-sharp-ninth": ("dominant-seventh", "#9"), + "dominant-seventh-sharp-eleventh": ("dominant-seventh", "#11"), + "dominant-seventh-flat-thirteenth": ("dominant-seventh", "b13"), + "dominant-seventh-added-thirteenth": ("dominant-seventh", "13"), + "dominant-ninth-sharp-eleventh": ("dominant-ninth", "#11"), + "major-seventh-sharp-eleventh": ("major-seventh", "#11"), + "major-seventh-added-sixth": ("major-seventh", "6"), + "major-sixth-added-ninth": ("major-sixth", "9"), + "minor-sixth-added-ninth": ("minor-sixth", "9"), + "major-added-ninth": ("major", "9"), + "minor-added-ninth": ("minor", "9"), + "minor-seventh-added-eleventh": ("minor-seventh", "11"), +} # Qualities TiLiA keeps in root position, whatever their intervals. _NO_INVERSION_QUALITIES = frozenset( {"Italian", "French", "German", "Neapolitan", "Tristan", "power"} @@ -312,6 +331,11 @@ def color(value: Any) -> Outcome: return _broken(not_valid("color", value)) +def split_added_tone(quality: str) -> tuple[str, str]: + """A quality's base quality and added tone, or the quality and "".""" + return ADDED_TONE_QUALITIES.get(quality, (quality, "")) + + def not_valid(attribute: str, value: Any) -> str: """The reason for a value of `attribute` that isn't one, for every kind. @@ -412,9 +436,13 @@ def _music21() -> tuple[ModuleType | None, Exception | None]: def _quality_and_inversion( music21: ModuleType, attrs: Mapping[str, Any] ) -> Outcome | None: - """The quality is one of music21's `CHORD_TYPES`, and the inversion one it has.""" + """The quality is one of music21's `CHORD_TYPES` or of TiLiA's added-tone + qualities, and the inversion one it has.""" quality = attrs.get("quality", "major") - if not (isinstance(quality, str) and quality in music21.harmony.CHORD_TYPES): + if not ( + isinstance(quality, str) + and split_added_tone(quality)[0] in music21.harmony.CHORD_TYPES + ): return _broken(not_valid("quality", quality)) inversion = attrs.get("inversion", 0) if not (_is_integer(inversion) and 0 <= inversion <= _inversions(music21, quality)): @@ -423,7 +451,9 @@ def _quality_and_inversion( def _inversions(music21: ModuleType, quality: str) -> int: - """A quality's inversions, as TiLiA's `get_inversion_amount` counts them.""" + """A quality's inversions, as TiLiA's `get_inversion_amount` counts them: an + added tone isn't one the chord can be inverted to.""" + quality, _ = split_added_tone(quality) if quality in _NO_INVERSION_QUALITIES: return 0 intervals = str(music21.harmony.CHORD_TYPES[quality][0]) diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 5d1484004..877139f1b 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -677,6 +677,37 @@ def test_inversions_as_tilia_counts_them(self, quality, inversions): f"{inversions + 1} is not a valid inversion.", ) + @pytest.mark.parametrize( + "quality, base, tone, inversions", + [ + ("dominant-seventh-flat-ninth", "dominant-seventh", "b9", 3), + ("dominant-seventh-sharp-ninth", "dominant-seventh", "#9", 3), + ("dominant-seventh-sharp-eleventh", "dominant-seventh", "#11", 3), + ("dominant-seventh-flat-thirteenth", "dominant-seventh", "b13", 3), + ("dominant-seventh-added-thirteenth", "dominant-seventh", "13", 3), + ("dominant-ninth-sharp-eleventh", "dominant-ninth", "#11", 4), + ("major-seventh-sharp-eleventh", "major-seventh", "#11", 3), + ("major-seventh-added-sixth", "major-seventh", "6", 3), + ("major-sixth-added-ninth", "major-sixth", "9", 3), + ("minor-sixth-added-ninth", "minor-sixth", "9", 3), + ("major-added-ninth", "major", "9", 2), + ("minor-added-ninth", "minor", "9", 2), + ("minor-seventh-added-eleventh", "minor-seventh", "11", 3), + ], + ) + def test_the_added_tone_qualities_tilia_has(self, quality, base, tone, inversions): + # An added tone isn't one the chord can be inverted to: the base quality's + # inversions count. + assert rules.split_added_tone(quality) == (base, tone) + assert check("harmony", quality=quality, inversion=inversions) == OK + assert check("harmony", quality=quality, inversion=inversions + 1) == ( + False, + f"{inversions + 1} is not a valid inversion.", + ) + + def test_a_quality_without_an_added_tone_is_its_own_base(self): + assert rules.split_added_tone("major") == ("major", "") + def test_the_inversion_of_the_default_quality(self): assert check("harmony", inversion=2) == OK From 5c67ed895cb661ff5c8ad015b8923bc6aa49d8a3 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 26 Oct 2026 23:44:03 +0100 Subject: [PATCH 34/37] fix(core): a hierarchy accepts a media length too large for a float _hierarchy_times turned the media length into a float before comparing, so rules.hierarchy(10**400, ...) raised OverflowError where marker and range_ accept the same length. It compares the values as they are given, as _point does, and turns them into floats only in the reasons that show them, as TiLiA does. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 20 ++++++++++++++------ packages/tilia-core/tests/test_rules.py | 19 ++++++++++++++++--- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 5f647e46f..3299b5c98 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -469,18 +469,26 @@ def _mode_type(music21: ModuleType, attrs: Mapping[str, Any]) -> Outcome | None: def _hierarchy_times(start: float, end: float, media_length: float) -> Outcome | None: - # TiLiA shows these as floats, whatever they were given as. - start, end, media_length = float(start), float(end), float(media_length) + # The values are compared as they are given, as `_point` compares them, so a + # media length too large for a float is longer than any time. TiLiA shows them + # as floats, and so do the reasons. if start > media_length: return _broken( - f"Start time '{start}' is bigger than media time '{media_length}'" + f"Start time '{float(start)}' is bigger than media time" + f" '{float(media_length)}'" ) if end > media_length: - return _broken(f"End time '{end}' is bigger than media time '{media_length}'") + return _broken( + f"End time '{float(end)}' is bigger than media time" + f" '{float(media_length)}'" + ) if start < 0: - return _broken(f"Start time can't be negative. Got '{start}'") + return _broken(f"Start time can't be negative. Got '{float(start)}'") if end <= start: - return _broken(f"End time '{end}' should be bigger than start time '{start}'") + return _broken( + f"End time '{float(end)}' should be bigger than start time" + f" '{float(start)}'" + ) return None diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 877139f1b..4da334908 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -316,6 +316,10 @@ def test_an_integer_too_large_for_a_float(self): assert check("marker", time=huge) == (False, f"{huge} is not a valid time.") assert check("hierarchy", end=huge) == (False, f"{huge} is not a valid end.") + @pytest.mark.parametrize("kind", [*POINT_KINDS, "hierarchy", "range"]) + def test_a_media_length_too_large_for_a_float(self, kind): + assert check(kind, media_length=10**400) == OK + def test_an_integer_too_long_to_show(self): if not hasattr(sys, "set_int_max_str_digits"): pytest.skip("this Python shows integers of any length") @@ -850,10 +854,19 @@ def test_an_end_not_after_its_start(self, end): f"End time '{end}' should be bigger than start time '2.0'", ) - def test_numbers_given_as_integers_read_as_tilias_floats(self): - assert check("hierarchy", start=61, end=62, media_length=60) == ( + @pytest.mark.parametrize( + "start, end, reason", + [ + (61, 62, "Start time '61.0' is bigger than media time '60.0'"), + (1, 61, "End time '61.0' is bigger than media time '60.0'"), + (-1, 2, "Start time can't be negative. Got '-1.0'"), + (2, 1, "End time '1.0' should be bigger than start time '2.0'"), + ], + ) + def test_numbers_given_as_integers_read_as_tilias_floats(self, start, end, reason): + assert check("hierarchy", start=start, end=end, media_length=60) == ( False, - "Start time '61.0' is bigger than media time '60.0'", + reason, ) @pytest.mark.parametrize("level", [0, -1]) From 9928285cad9c82885872d1430b286df7de405cd2 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 26 Oct 2026 23:46:09 +0100 Subject: [PATCH 35/37] test(core): test both parts of the hierarchy boundaries' tolerance dev's _times_close, since 2ceec157, is math.isclose with abs_tol=1e-9 and the default rel_tol of 1e-9: two times count as the same within 1e-9, or within a billionth of the larger, whichever is more. Every boundary the tests used was at 1 s or later, where the relative part alone covers 1e-13, so they passed with abs_tol=0. Cases near 0, at 0.5 s and at an hour, met and crossing, pin both parts, and the sweep against get_boundary_conflicts now covers boundaries from 0 to 4 s, moved by up to 2e-9. The comments that said "less than 1e-9 apart" say what isclose compares. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 15 ++++-- packages/tilia-core/tests/test_rules.py | 59 +++++++++++++++++---- 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 3299b5c98..3cdae34a0 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -70,9 +70,12 @@ {"Italian", "French", "German", "Neapolitan", "Tristan", "power"} ) -# Hierarchy boundaries less than this far apart count as the same, as TiLiA's -# `get_boundary_conflicts` compares them since `dev`'s 2ceec157: files TiLiA saved -# before then hold boundaries a few ulps apart where they should meet. +# Two hierarchy boundaries count as the same when they are within 1e-9 s of each +# other, or within a billionth of the larger, whichever is more: `math.isclose` +# with this `abs_tol` and its default `rel_tol` of 1e-9, as TiLiA's +# `get_boundary_conflicts` compares them since `dev`'s 2ceec157. So below 1 s they +# meet within 1e-9 s, and at an hour within 3.6e-6 s. Files TiLiA saved before then +# hold boundaries a few ulps apart where they should meet. _TIME_ABS_TOL = 1e-9 # The names TiLiA's reasons give the point kinds (their `frontend_name`). @@ -559,7 +562,8 @@ def _cross(start: float, end: float, level: int, other: Mapping[str, Any]) -> bo """Whether two hierarchies overlap, and neither lies inside the other, lower. This is when TiLiA's `get_boundary_conflicts` finds a conflict between them, as - `dev` has it since 2ceec157: times less than 1e-9 apart count as the same. + `dev` has it since 2ceec157: two boundaries `_times_close` finds close count as + the same. """ other_start, other_end = other.get("start"), other.get("end") other_level = other.get("level") @@ -577,7 +581,8 @@ def _cross(start: float, end: float, level: int, other: Mapping[str, Any]) -> bo def _times_close(a: float, b: float) -> bool: - """Whether two times count as the same, as TiLiA's `_times_close` says.""" + """Whether two times count as the same, as TiLiA's `_times_close` says: within + `_TIME_ABS_TOL`, or within a billionth of the larger, whichever is more.""" try: return math.isclose(a, b, abs_tol=_TIME_ABS_TOL) except OverflowError: # an integer too large for a float is close only to itself diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 4da334908..ba91b24e2 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -152,8 +152,9 @@ def tilia_creates_range( return True -# `dev`'s comparisons of hierarchy boundaries since 2ceec157: times less than -# 1e-9 apart count as the same. +# `dev`'s comparisons of hierarchy boundaries since 2ceec157: two times count as +# the same when they are within 1e-9 of each other, or within a billionth of the +# larger, whichever is more (`math.isclose`'s default `rel_tol` is 1e-9). def _times_close(a, b): @@ -1030,27 +1031,63 @@ def test_nesting_is_tilias_boundary_conflicts(self): conflicts = tilia_boundary_conflicts([existing, candidate]) assert outcome.ok is not bool(conflicts), (existing, candidate) - def test_boundaries_a_few_ulps_apart_meet(self): - # As in files TiLiA saved before it rescaled subtrees in one step. - others = [hierarchy(0.0, 1.0, 1)] + @pytest.mark.parametrize( + "existing, candidate", + [ + # As in files TiLiA saved before it rescaled subtrees in one step. + ((0.0, 1.0, 1), (1.0 - 1e-13, 2.0, 1)), + # Below 1 s, boundaries within 1e-9 meet. + ((1e-13, 1.0, 2), (0.0, 1.0, 1)), + ((0.0, 0.5, 1), (0.5 - 9e-10, 1.0, 1)), + # Above it, boundaries within a billionth of the larger. + ((0.0, 3600.0, 1), (3600.0 - 2e-6, 3601.0, 1)), + ], + ) + def test_close_boundaries_meet(self, existing, candidate): + start, end, level = candidate + others = [hierarchy(*existing)] + + outcome = check( + "hierarchy", others, media_length=7200.0, start=start, end=end, level=level + ) - assert check("hierarchy", others, start=1.0 - 1e-13, end=2.0, level=1) == OK + assert outcome == OK + + @pytest.mark.parametrize( + "existing, candidate", + [ + ((0.0, 0.5, 1), (0.5 - 2e-9, 1.0, 1)), + ((0.0, 3600.0, 1), (3600.0 - 5e-6, 3601.0, 1)), + ], + ) + def test_boundaries_further_apart_cross(self, existing, candidate): + start, end, level = candidate + others = [hierarchy(*existing)] + + outcome = check( + "hierarchy", others, media_length=7200.0, start=start, end=end, level=level + ) + + assert outcome == (False, "Component would overlap with existing component.") def test_nesting_with_float_noise_is_tilias_boundary_conflicts(self): - # Every pair of spans from 1 to 4 s at levels 1 to 3, with each boundary - # moved by up to 1e-13. + # Every pair of spans with boundaries from 0 to 4 s at levels 1 to 3, with + # each boundary moved by up to 2e-9, and none before 0. Below 1 s, 1e-9 is + # more than a billionth of a time, and above it less, so the sweep compares + # both parts of the tolerance. noise = random.Random(2026) + moves = (0, 1e-13, -1e-13, 5e-10, -5e-10, 2e-9, -2e-9) spans = [ (start, end, level) - for start, end in itertools.combinations(range(1, 5), 2) + for start, end in itertools.combinations((0.0, 0.25, 0.5, 1.0, 2.0, 4.0), 2) for level in (1, 2, 3) ] for existing, candidate in itertools.product(spans, repeat=2): for _ in range(4): moved = [ ( - start + noise.choice((-1e-13, 0, 1e-13)), - end + noise.choice((-1e-13, 0, 1e-13)), + max(0.0, start + noise.choice(moves)), + end + noise.choice(moves), level, ) for start, end, level in (existing, candidate) From 0313ecbef8c9006087fd6f67c01e70bd5253599b Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 26 Oct 2026 23:46:57 +0100 Subject: [PATCH 36/37] fix(core): an outcome's others can't be changed Every outcome that keeps to the rules is the one OK, so after rules.OK.others = ("zz",) each of them carried ("zz",). others is a read-only property now, over a private attribute: a tuple's subclass can't have a slot. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 12 +++++++++--- packages/tilia-core/tests/test_rules.py | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 3cdae34a0..17926f84c 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -112,11 +112,13 @@ class Outcome(tuple[bool, str]): time or place, the hierarchies it crosses, the ranges it should join. """ - others: tuple[str, ...] + # Read through `others`, which can't be set: every outcome that keeps to the + # rules is the one `OK`. A tuple's subclass can't give it a slot. + _others: tuple[str, ...] def __new__(cls, ok: bool, reason: str = "", others: Iterable[str] = ()) -> Outcome: outcome = super().__new__(cls, (bool(ok), reason)) - outcome.others = tuple(others) + outcome._others = tuple(others) return outcome def __bool__(self) -> bool: @@ -124,7 +126,11 @@ def __bool__(self) -> bool: def __getnewargs__(self) -> tuple[bool, str, tuple[str, ...]]: # type: ignore[override] # What copy and pickle give __new__: a tuple's would be the pair alone. - return self[0], self[1], self.others + return self[0], self[1], self._others + + @property + def others(self) -> tuple[str, ...]: + return self._others @property def ok(self) -> bool: diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index ba91b24e2..14f94ab9c 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -272,6 +272,21 @@ def test_a_broken_rule_names_the_other_components(self): assert outcome.others == ("a", "b") assert (outcome.ok, outcome.reason) == (False, "Component would overlap") + @pytest.mark.parametrize( + "outcome", + [rules.Outcome(True), rules.Outcome(False, "Invalid row ID.", ["a"])], + ) + def test_its_others_cant_be_changed(self, outcome): + # Every outcome that keeps to the rules is the one `OK`, so changing its + # others would change them all. The test changes new outcomes, so that it + # leaves `OK` as it is if it fails. + others = outcome.others + + with pytest.raises(AttributeError): + outcome.others = ("zz",) + + assert outcome.others == others + class TestEveryKind: @pytest.mark.parametrize( From 08463e8386164084f9690b34c032384ae1a7e45d Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 26 Oct 2026 23:48:06 +0100 Subject: [PATCH 37/37] fix(core): name a value that can't be shown by its type _shown called any value Python won't turn into text "an integer too long to show", so a label of [10**5000] gave "an integer too long to show is not a valid label.", though the label is a list. An integer keeps that text; anything else is named by its type: "a list that can't be shown". Only a caller in Python can give such a value. Part of TimeLineAnnotator/project-management#291 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/rules.py | 9 +++- packages/tilia-core/tests/test_rules.py | 56 ++++++++++++++------- 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/rules.py b/packages/tilia-core/src/tilia_core/rules.py index 17926f84c..96e9fc753 100644 --- a/packages/tilia-core/src/tilia_core/rules.py +++ b/packages/tilia-core/src/tilia_core/rules.py @@ -725,11 +725,16 @@ def _is_row_id(value: Any) -> bool: def _shown(value: Any) -> str: """`value` as a reason shows it. An integer Python won't turn into text, one - of more than 4,300 digits unless set otherwise, is named instead.""" + of more than 4,300 digits unless set otherwise, is named instead, and so is a + value holding one, by its type: "a list that can't be shown".""" try: return f"{value}" except ValueError: - return "an integer too long to show" + if isinstance(value, int): + return "an integer too long to show" + name = type(value).__name__ + article = "an" if name[:1].lower() in "aeiou" else "a" + return f"{article} {name} that can't be shown" def _broken(reason: str) -> Outcome: diff --git a/packages/tilia-core/tests/test_rules.py b/packages/tilia-core/tests/test_rules.py index 14f94ab9c..fca4edd1f 100644 --- a/packages/tilia-core/tests/test_rules.py +++ b/packages/tilia-core/tests/test_rules.py @@ -1,3 +1,4 @@ +import collections import copy import importlib.util import itertools @@ -68,6 +69,17 @@ def music21(): return pytest.importorskip("music21", reason="needs the harmony extra") +@pytest.fixture +def default_int_digits(): + # Python's default limit on the digits of an integer it turns into text. + if not hasattr(sys, "set_int_max_str_digits"): + pytest.skip("this Python shows integers of any length") + limit = sys.get_int_max_str_digits() + sys.set_int_max_str_digits(4300) + yield + sys.set_int_max_str_digits(limit) + + @pytest.fixture def without_music21(monkeypatch): find_spec = importlib.util.find_spec @@ -336,24 +348,32 @@ def test_an_integer_too_large_for_a_float(self): def test_a_media_length_too_large_for_a_float(self, kind): assert check(kind, media_length=10**400) == OK - def test_an_integer_too_long_to_show(self): - if not hasattr(sys, "set_int_max_str_digits"): - pytest.skip("this Python shows integers of any length") - limit = sys.get_int_max_str_digits() - sys.set_int_max_str_digits(4300) - try: - huge = 10**5000 - - assert check("marker", time=huge) == ( - False, - "an integer too long to show is not a valid time.", - ) - assert check("hierarchy", level=-huge) == ( - False, - "Level can't be below 1. Got 'an integer too long to show'", - ) - finally: - sys.set_int_max_str_digits(limit) + def test_an_integer_too_long_to_show(self, default_int_digits): + huge = 10**5000 + + assert check("marker", time=huge) == ( + False, + "an integer too long to show is not a valid time.", + ) + assert check("hierarchy", level=-huge) == ( + False, + "Level can't be below 1. Got 'an integer too long to show'", + ) + + @pytest.mark.parametrize( + "value, shown", + [ + ([10**5000], "a list"), + ((10**5000,), "a tuple"), + ({"a": 10**5000}, "a dict"), + (collections.OrderedDict(a=10**5000), "an OrderedDict"), + ], + ) + def test_another_value_that_cant_be_shown(self, default_int_digits, value, shown): + assert check("marker", label=value) == ( + False, + f"{shown} that can't be shown is not a valid label.", + ) def test_a_missing_time(self): assert rules.marker(MEDIA, [], {}) == (False, "None is not a valid time.")