From eca899e4fa8b30d8a207f4436df4dfff5f6b4b8d Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:30:50 +0200 Subject: [PATCH 01/27] feat(core): choose a document's time map by role find_time_map(document, roles=None) returns a TimeMapFound: the time map of the beat timeline whose role is time_map, else of the first beat timeline by ordinal (guessed), with the chosen timeline's id, the reason when it gives no time map, and warnings. The roles are the caller's when given, else each timeline's metadata["role"]. When several beat timelines claim the role, the first by ordinal wins, with a warning. No other timeline stands in for a chosen one that gives no time map. time_map_of is find_time_map(...).time_map, and measure_rows(document, timeline_id) gives a beat timeline's computed table, folded tables included. The warning that the last beat is at or after the end of the media moves from _closing_barline, which logged it each time a table was built, into TimeMapFound.warnings; find_time_map logs it once. test_timemap_prototype.py's test_choosing_the_time_map runs without its strict xfail, now that find_time_map exists. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/__init__.py | 110 ++++++- .../src/tilia_core/timemap/table.py | 41 +-- .../src/tilia_core/timemap/timemap.py | 7 + .../tests/timemap/test_timemap_choose.py | 308 ++++++++++++++++++ .../tests/timemap/test_timemap_positions.py | 11 +- .../tests/timemap/test_timemap_prototype.py | 16 +- .../tests/timemap/test_timemap_table.py | 4 +- 7 files changed, 457 insertions(+), 40 deletions(-) create mode 100644 packages/tilia-core/tests/timemap/test_timemap_choose.py diff --git a/packages/tilia-core/src/tilia_core/timemap/__init__.py b/packages/tilia-core/src/tilia_core/timemap/__init__.py index f38bf1ab0..96b746b8e 100644 --- a/packages/tilia-core/src/tilia_core/timemap/__init__.py +++ b/packages/tilia-core/src/tilia_core/timemap/__init__.py @@ -2,11 +2,102 @@ from __future__ import annotations +import logging +from collections.abc import Mapping +from dataclasses import dataclass + from tilia_core import tla -from tilia_core.timemap.table import MeasureRow -from tilia_core.timemap.timemap import Positions, TimeMap, build_time_map +from tilia_core.timemap.table import MeasureRow, build_rows +from tilia_core.timemap.timemap import ( + Positions, + TimeMap, + build_time_map, + media_warning, +) + +__all__ = [ + "MeasureRow", + "Positions", + "TimeMap", + "TimeMapFound", + "find_time_map", + "measure_rows", + "time_map_for", + "time_map_of", +] + +logger = logging.getLogger(__name__) + +TIME_MAP_ROLE = "time_map" +TWO_CLAIM = "two timelines claim the role time_map; the first by ordinal is used" + + +@dataclass(frozen=True) +class TimeMapFound: + """A document's time map as `find_time_map` chose it, or why there is none.""" + + time_map: TimeMap | None + timeline_id: str | None # the chosen beat timeline, even when it gives no time map + guessed: bool # no timeline had the role time_map: the first beat timeline was taken + reason: str | None # why the chosen timeline gives no time map + warnings: list[str] # several claim the role; the last beat past the media + + +def find_time_map( + document: tla.Document, roles: Mapping[str, str | None] | None = None +) -> TimeMapFound: + """The document's time map: its beat timeline whose role is time_map, else + its first beat timeline by ordinal, guessed. + + `roles` gives each timeline's role by its id, as the caller resolved them + (the index: from the file and the corpus's manifest), and a timeline it + leaves out has none; without it, the roles are the file's. When several + beat timelines have the role, the first by ordinal wins, with a warning. + When the chosen timeline gives no time map, no other stands in: the file + has no bars, and `reason` says why. A document without a beat timeline has + no time map, and nothing is wrong with it: no reason, no warning. -__all__ = ["MeasureRow", "Positions", "TimeMap", "time_map_for"] + The warning that the last beat is at or after the end of the media is + logged here, once for each call, and not when a table is built. + """ + beat_timelines = sorted( + (t for t in document.timelines.values() if t.kind == "beat"), + key=lambda t: t.ordinal, + ) + if not beat_timelines: + return TimeMapFound( + time_map=None, timeline_id=None, guessed=False, reason=None, warnings=[] + ) + claiming = [t for t in beat_timelines if _role(t, roles) == TIME_MAP_ROLE] + warnings = [TWO_CLAIM] if len(claiming) > 1 else [] + timeline = (claiming or beat_timelines)[0] + guessed = not claiming + time_map, reason = build_time_map(document, timeline, guessed=guessed) + if time_map is not None: + warning = media_warning(document, time_map) + if warning is not None: + logger.warning("Timeline %s: %s.", timeline.id, warning) + warnings.append(warning) + return TimeMapFound( + time_map=time_map, + timeline_id=timeline.id, + guessed=guessed, + reason=reason, + warnings=warnings, + ) + + +def _role(timeline: tla.Timeline, roles: Mapping[str, str | None] | None) -> object: + if roles is None: + return timeline.metadata.get("role") + return roles.get(timeline.id) + + +def time_map_of( + document: tla.Document, roles: Mapping[str, str | None] | None = None +) -> TimeMap | None: + """The document's time map, chosen as `find_time_map` chooses it; None when it has none.""" + return find_time_map(document, roles).time_map def time_map_for(document: tla.Document, timeline_id: str) -> TimeMap | None: @@ -16,3 +107,16 @@ def time_map_for(document: tla.Document, timeline_id: str) -> TimeMap | None: return None time_map, _ = build_time_map(document, timeline) return time_map + + +def measure_rows(document: tla.Document, timeline_id: str) -> list[MeasureRow]: + """The measure table of the beat timeline `timeline_id`, in its order. + + Also for a folded table, which has no time map. No rows while a beat has no + usable time or the table is broken, and none for a timeline that isn't a + beat timeline. + """ + timeline = document.timelines.get(timeline_id) + if timeline is None or timeline.kind != "beat": + return [] + return build_rows(document, timeline) diff --git a/packages/tilia-core/src/tilia_core/timemap/table.py b/packages/tilia-core/src/tilia_core/timemap/table.py index 56ad100a2..5cb9e0b66 100644 --- a/packages/tilia-core/src/tilia_core/timemap/table.py +++ b/packages/tilia-core/src/tilia_core/timemap/table.py @@ -3,7 +3,6 @@ from __future__ import annotations -import logging import math from collections.abc import Iterable, Mapping, Sequence from dataclasses import dataclass, field @@ -15,8 +14,6 @@ from tilia_core.timemap.meters import meters_of from tilia_core.timemap.units import MeasureMeter, format_units -logger = logging.getLogger(__name__) - class FrozenMetadata(dict[str, "str | tuple[str, ...]"]): """A measure's metadata, read-only, with its lists as tuples. @@ -168,7 +165,7 @@ def build_rows_from( return [] folded = is_folded(rows) ends = [row.beat_times[0] for row in rows[1:]] - ends.append(_closing_barline(document, timeline.id, rows)) + ends.append(_closing_barline(document, rows)) passes: dict[int, int] = {} movement = 1 out: list[MeasureRow] = [] @@ -229,28 +226,34 @@ def is_folded(rows: Sequence[RowIn]) -> bool: return any(row.next is not None for row in rows) -def _closing_barline( - document: tla.Document, timeline_id: str, rows: Sequence[RowIn] -) -> float: +def _closing_barline(document: tla.Document, rows: Sequence[RowIn]) -> float: # The last beat lasts as long as the gap before it, or until the end of the # media if that comes sooner. No beats are added to the last measure. *before, last = [t for row in rows[-2:] for t in row.beat_times][-2:] end = last + (last - before[0] if before else 0.0) - media_length = document.media_length - if media_length is None or document.time_unit != "seconds": - return end # the media's length is in seconds - if media_length <= last: - logger.warning( - "Timeline %s: the last beat (%s) is at or after the end of the media (%s); " - "the last measure ends one gap after it.", - timeline_id, - last, - media_length, - ) - return end + media_length = _media_length(document) + if media_length is None or media_length <= last: + return end # past the media, the gap alone; find_time_map warns (past_media) return min(end, media_length) +def past_media(document: tla.Document, last_beat: float) -> str | None: + """The warning that the last beat is at or after the end of the media, so + that the last measure ends one gap after it; None when it isn't.""" + media_length = _media_length(document) + if media_length is None or media_length > last_beat: + return None + return ( + f"the last beat ({last_beat}) is at or after the end of the media " + f"({media_length}); the last measure ends one gap after it" + ) + + +def _media_length(document: tla.Document) -> float | None: + # The media's length is in seconds: it doesn't cap a quarters file. + return document.media_length if document.time_unit == "seconds" else None + + def find_rows( rows: Sequence[MeasureRow], number: int | None = None, diff --git a/packages/tilia-core/src/tilia_core/timemap/timemap.py b/packages/tilia-core/src/tilia_core/timemap/timemap.py index ca2b0e079..1fe0f77ab 100644 --- a/packages/tilia-core/src/tilia_core/timemap/timemap.py +++ b/packages/tilia-core/src/tilia_core/timemap/timemap.py @@ -21,6 +21,7 @@ build_rows_from, is_folded, missing_time, + past_media, ) TOLERANCE = 0.1 # in the file's unit: a time this close to a beat is on it @@ -207,3 +208,9 @@ def build_time_map( _times=tuple(times), ) return time_map, None + + +def media_warning(document: tla.Document, time_map: TimeMap) -> str | None: + """The warning that the map's last beat is at or after the end of the + media, which leaves the closing barline one gap after it; else None.""" + return past_media(document, time_map._times[-1]) diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py new file mode 100644 index 000000000..00d162b99 --- /dev/null +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -0,0 +1,308 @@ +import dataclasses +import logging + +import pytest + +import tilia_core.timemap as timemap +from docs import beat_timeline, document, pickup_piece +from tilia_core import tla +from tilia_core.timemap import ( + TimeMapFound, + find_time_map, + measure_rows, + time_map_for, + time_map_of, +) +from tilia_core.timemap.timemap import FOLDED + +TWO_CLAIM = "two timelines claim the role time_map; the first by ordinal is used" + + +def _beats(ordinal: int, role: str | None = None, *, last: int = 3) -> tla.Timeline: + """2/4, a beat every second from 0 to `last`.""" + return beat_timeline( + [ + {"time": float(t), "measure": {} if t % 2 == 0 else None} + for t in range(last + 1) + ], + ordinal=ordinal, + role=role, + ) + + +def _other(kind: str, ordinal: int, **kwargs) -> tla.Timeline: + return tla.Timeline(id=tla.new_id(), kind=kind, ordinal=ordinal, **kwargs) + + +def _folded(ordinal: int = 1, role: str | None = None) -> tla.Timeline: + timeline = _beats(ordinal, role) + timeline.measures.rows[1].next = [timeline.measures.rows[0].id] + return timeline + + +def test_the_timeline_with_the_role_is_the_time_map(): + # Spec, US4 scenario 1. + first, second = _beats(1), _beats(2, role="time_map") + + found = find_time_map(document(first, second)) + + assert found.timeline_id == second.id + assert found.time_map is not None + assert found.time_map.timeline_id == second.id + assert not found.guessed and not found.time_map.guessed + assert found.reason is None and found.warnings == [] + + +def test_without_the_role_the_first_beat_timeline_by_ordinal_is_guessed(): + # Spec, US4 scenario 2: by ordinal, not by the order of the file; other + # kinds don't count. + markers = _other("marker", 1) + later, earlier = _beats(5), _beats(3) + + found = find_time_map(document(markers, later, earlier)) + + assert found.timeline_id == earlier.id + assert found.time_map.timeline_id == earlier.id + assert found.guessed and found.time_map.guessed + assert found.reason is None and found.warnings == [] + + +@pytest.mark.parametrize("claimants", [2, 3]) +def test_when_several_claim_the_role_the_first_by_ordinal_wins(claimants): + # Spec, US4 scenario 3: one warning, however many claim it. + unclaimed = _beats(1) + claiming = [_beats(10 - j, role="time_map") for j in range(claimants)] + + found = find_time_map(document(unclaimed, *claiming)) + + assert found.timeline_id == claiming[-1].id + assert found.time_map.timeline_id == claiming[-1].id + assert not found.guessed + assert found.warnings == [TWO_CLAIM] + + +def test_the_callers_roles_count_not_only_the_files(): + # Spec, US4 scenario 4: the roles WP12 resolved from the file and the manifest. + in_file, in_manifest = _beats(1, role="time_map"), _beats(2) + doc = document(in_file, in_manifest) + roles = {in_file.id: None, in_manifest.id: "time_map"} + + found = find_time_map(doc, roles) + + assert found.timeline_id == in_manifest.id and not found.guessed + assert found.warnings == [] + assert time_map_of(doc, roles).timeline_id == in_manifest.id + assert time_map_of(doc).timeline_id == in_file.id + + +@pytest.mark.parametrize("roles", [{}, "none for each"]) +def test_the_callers_roles_replace_the_files(roles): + first, second = _beats(1), _beats(2, role="time_map") + if roles == "none for each": + roles = {first.id: None, second.id: None} + + found = find_time_map(document(first, second), roles) + + assert found.timeline_id == first.id and found.guessed + + +def test_only_a_beat_timeline_can_have_the_role(): + markers = _other("marker", 1, metadata={"role": "time_map"}) + beats = _beats(2) + + for roles in (None, {markers.id: "time_map"}): + found = find_time_map(document(markers, beats), roles) + + assert found.timeline_id == beats.id and found.guessed + + +def test_a_timeline_that_names_a_beat_timeline_gets_its_table_and_time_map(): + # Spec, US4 scenario 5: a score timeline names its table (WP3's FR-133). + default, named = _beats(1, role="time_map"), _beats(2, last=7) + score = _other("score", 3, attrs={"measure_table": named.id}) + doc = document(default, named, score) + + time_map = time_map_for(doc, score.attrs["measure_table"]) + rows = measure_rows(doc, score.attrs["measure_table"]) + + assert time_map.timeline_id == named.id and not time_map.guessed + assert [row.number for row in rows] == [1, 2, 3, 4] + assert rows == time_map.measure_rows() + assert find_time_map(doc).timeline_id == default.id + + +def _one_beat(**kwargs) -> tla.Timeline: + return beat_timeline([{"time": 0.0, "measure": {}}], **kwargs) + + +def _same_time(**kwargs) -> tla.Timeline: + return beat_timeline( + [{"time": 0.0, "measure": {}}, {"time": 0.0}, {"time": 1.0}], **kwargs + ) + + +@pytest.mark.parametrize( + "make, reason", + [ + (_one_beat, "fewer than two beats"), + (_same_time, "two beats at the same time (0.0)"), + (_folded, FOLDED), + ], +) +@pytest.mark.parametrize("role", ["time_map", None]) +def test_no_other_timeline_stands_in_for_one_without_a_time_map(make, reason, role): + # Spec, US4 scenario 6, and FR-003: the file has no bars, and says why. + chosen, other = make(ordinal=1, role=role), _beats(2) + doc = document(chosen, other) + + found = find_time_map(doc) + + assert found.time_map is None + assert found.timeline_id == chosen.id + assert found.guessed is (role is None) + assert found.reason == reason + assert found.warnings == [] + assert time_map_of(doc) is None + + +def test_a_document_without_beat_timelines_has_no_time_map(caplog): + for doc in (document(), document(_other("marker", 1), _other("hierarchy", 2))): + found = find_time_map(doc) + + assert found == TimeMapFound( + time_map=None, timeline_id=None, guessed=False, reason=None, warnings=[] + ) + assert time_map_of(doc) is None + assert not caplog.records + + +def test_found_is_frozen(): + found = find_time_map(document(_beats(1))) + + with pytest.raises(dataclasses.FrozenInstanceError): + found.guessed = False # type: ignore[misc] + + +@pytest.mark.parametrize("media_length", [19.5, 20.0]) +def test_the_last_beat_at_or_after_the_end_of_the_media_is_warned_once( + media_length, caplog +): + # FR-082: returned, and logged with the timeline's id, once per file. + timeline = pickup_piece() # the last beat at 20 s + doc = document(timeline, media_length=media_length) + + with caplog.at_level(logging.WARNING): + found = find_time_map(doc) + + assert found.time_map.end == 21.0 + (warning,) = found.warnings + assert warning == ( + f"the last beat (20.0) is at or after the end of the media ({media_length}); " + "the last measure ends one gap after it" + ) + (record,) = caplog.records + assert record.name == "tilia_core.timemap" + assert record.levelno == logging.WARNING + assert timeline.id in record.getMessage() + assert warning in record.getMessage() + + +def test_the_warnings_come_in_the_order_they_were_found(): + claiming = [_beats(j, role="time_map") for j in (1, 2)] + + found = find_time_map(document(*claiming, media_length=2.0)) + + assert len(found.warnings) == 2 + assert found.warnings[0] == TWO_CLAIM + assert found.warnings[1].startswith("the last beat (3.0)") + + +@pytest.mark.parametrize( + "media_length, time_unit", + [(None, "seconds"), (20.4, "seconds"), (30.0, "seconds"), (19.5, "quarters")], +) +def test_no_media_warning_when_the_media_lasts_past_the_last_beat( + media_length, time_unit, caplog +): + # A quarters file's times aren't in the media's seconds. + doc = document(pickup_piece(), media_length=media_length, time_unit=time_unit) + + with caplog.at_level(logging.WARNING): + found = find_time_map(doc) + + assert found.time_map is not None and found.warnings == [] + assert not caplog.records + + +def test_no_media_warning_without_a_time_map(caplog): + # The table is folded, so it has no closing barline to warn about. + doc = document(_folded(), media_length=2.0) + + with caplog.at_level(logging.WARNING): + found = find_time_map(doc) + + assert found.time_map is None and found.warnings == [] + assert not caplog.records + + +def test_tables_and_named_time_maps_log_nothing_about_the_media(caplog): + # FR-082: only find_time_map logs it, so a table built many times doesn't. + timeline = pickup_piece() + doc = document(timeline, media_length=19.5) + + with caplog.at_level(logging.WARNING): + for _ in range(3): + assert time_map_for(doc, timeline.id) is not None + assert measure_rows(doc, timeline.id) + + assert not caplog.records + + +def test_measure_rows_of_a_folded_table(): + # FR-013: no time map, but the rows are computed, in the timeline's order. + timeline = _folded() + doc = document(timeline) + + rows = measure_rows(doc, timeline.id) + + assert time_map_for(doc, timeline.id) is None + assert [(row.number, row.start, row.end, row.folded) for row in rows] == [ + (1, 0.0, 2.0, True), + (2, 2.0, 4.0, True), + ] + + +def test_measure_rows_give_none_while_a_beat_has_no_usable_time(): + timeline = _beats(1) + beat = timeline.components[timeline.measures.rows[1].beats[1]] + beat.attrs["time"] = float("nan") + + assert measure_rows(document(timeline), timeline.id) == [] + + +def test_measure_rows_of_a_timeline_that_is_not_a_beat_timeline(): + # Even one that carries a table, as only a beat timeline should. + beats, markers = _beats(1), _other("marker", 2) + with_table = dataclasses.replace(_beats(3), kind="marker") + doc = document(beats, markers, with_table) + + assert measure_rows(doc, markers.id) == [] + assert measure_rows(doc, with_table.id) == [] + assert time_map_for(doc, with_table.id) is None + assert measure_rows(doc, tla.new_id()) == [] + assert len(measure_rows(doc, beats.id)) == 2 + + +def test_the_package_exports_the_contracts_names_that_exist(): + assert sorted(timemap.__all__) == [ + "MeasureRow", + "Positions", + "TimeMap", + "TimeMapFound", + "find_time_map", + "measure_rows", + "time_map_for", + "time_map_of", + ] + for name in timemap.__all__: + assert getattr(timemap, name) is not None diff --git a/packages/tilia-core/tests/timemap/test_timemap_positions.py b/packages/tilia-core/tests/timemap/test_timemap_positions.py index 071e2380e..116f91206 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_positions.py +++ b/packages/tilia-core/tests/timemap/test_timemap_positions.py @@ -4,6 +4,7 @@ import pytest +import tilia_core.timemap.timemap as timemap_module from docs import beat_timeline, document, pickup_piece from tilia_core import tla from tilia_core.timemap import TimeMap, time_map_for @@ -514,15 +515,17 @@ def test_timemap_guessed_is_kept(): assert time_map.guessed and reason is None -def test_timemap_folded_table_is_refused_before_its_rows_are_built(caplog): - # The media ends before the last beat, which building the rows would log. +def test_timemap_folded_table_is_refused_before_its_rows_are_built(monkeypatch): folded = _timeline({0: {"number": 1}, 2: {}}, last=3) folded.measures.rows[1].next = [folded.measures.rows[0].id] - time_map, reason = build_time_map(document(folded, media_length=2.5), folded) + def build_rows_from(*args): + raise AssertionError("the rows of a folded table were built") + + monkeypatch.setattr(timemap_module, "build_rows_from", build_rows_from) + time_map, reason = build_time_map(document(folded), folded) assert time_map is None and reason.startswith("the table lists the measures") - assert not caplog.records def test_timemap_derives_its_arrays_from_its_rows_and_beats(): diff --git a/packages/tilia-core/tests/timemap/test_timemap_prototype.py b/packages/tilia-core/tests/timemap/test_timemap_prototype.py index 91ad3d0b2..26d516d96 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_prototype.py +++ b/packages/tilia-core/tests/timemap/test_timemap_prototype.py @@ -35,24 +35,18 @@ from docs import beat_timeline, document from tilia_core import tla -from tilia_core.timemap import TimeMap, time_map_for +from tilia_core.timemap import TimeMap, find_time_map, time_map_for from tilia_core.timemap import timemap as timemap_module # The prototype's TimeMap.position, length, beat_seconds and bar_seconds are the -# core's position, length and unit_seconds, which aren't written yet. Both marks -# are strict: once the names exist, these tests fail until the mark is removed. +# core's position, length and unit_seconds, which aren't written yet. The mark is +# strict: once the names exist, these tests fail until it is removed. needs_distances = pytest.mark.xfail( raises=AttributeError, strict=True, reason="TimeMap.position, length and unit_seconds come with " "TimeLineAnnotator/project-management#215", ) -# The prototype's pick_time_map is the core's find_time_map, not written yet. -needs_find_time_map = pytest.mark.xfail( - raises=ImportError, - strict=True, - reason="find_time_map comes with TimeLineAnnotator/project-management#216", -) T44 = [1.0 + 0.5 * i for i in range(12)] # 4/4, bars 1-3: downbeats 1.0, 3.0, 5.0 @@ -430,10 +424,8 @@ def test_no_time_map(): assert time_map_for(doc, tla.new_id()) is None -@needs_find_time_map def test_choosing_the_time_map(): - from tilia_core.timemap import find_time_map - + """The prototype's pick_time_map, which is the core's find_time_map.""" # the first beat timeline by ordinal hierarchy = tla.Timeline(id=tla.new_id(), kind="hierarchy", ordinal=1) late = beat_timeline([], ordinal=5) diff --git a/packages/tilia-core/tests/timemap/test_timemap_table.py b/packages/tilia-core/tests/timemap/test_timemap_table.py index c5019ed11..c0d454490 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_table.py +++ b/packages/tilia-core/tests/timemap/test_timemap_table.py @@ -234,13 +234,13 @@ def test_the_end_of_the_media_caps_the_closing_barline(media_length, end, caplog @pytest.mark.parametrize("media_length", [19.5, 20.0]) def test_media_ending_at_or_before_the_last_beat_leaves_the_gap(media_length, caplog): + # find_time_map warns about it, once per file; a table built logs nothing (FR-082). timeline = pickup_piece() with caplog.at_level(logging.WARNING): rows = build_rows(document(timeline, media_length=media_length), timeline) assert rows[-1].end == 21.0 - assert timeline.id in caplog.text - assert "media" in caplog.text + assert not caplog.records def test_the_end_of_the_media_doesnt_cap_a_quarters_file(caplog): From f37a2a7196f1d9e851c38dbdc764ec66987d5a06 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:27:51 +0200 Subject: [PATCH 02/27] fix(core): make TimeMapFound's warnings a tuple, so it hashes A frozen result that held a list couldn't be hashed, and could be changed through its list. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/__init__.py | 6 ++--- .../tests/timemap/test_timemap_choose.py | 22 ++++++++++--------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/__init__.py b/packages/tilia-core/src/tilia_core/timemap/__init__.py index 96b746b8e..d0229ab00 100644 --- a/packages/tilia-core/src/tilia_core/timemap/__init__.py +++ b/packages/tilia-core/src/tilia_core/timemap/__init__.py @@ -40,7 +40,7 @@ class TimeMapFound: timeline_id: str | None # the chosen beat timeline, even when it gives no time map guessed: bool # no timeline had the role time_map: the first beat timeline was taken reason: str | None # why the chosen timeline gives no time map - warnings: list[str] # several claim the role; the last beat past the media + warnings: tuple[str, ...] # several claim the role; the last beat past the media def find_time_map( @@ -66,7 +66,7 @@ def find_time_map( ) if not beat_timelines: return TimeMapFound( - time_map=None, timeline_id=None, guessed=False, reason=None, warnings=[] + time_map=None, timeline_id=None, guessed=False, reason=None, warnings=() ) claiming = [t for t in beat_timelines if _role(t, roles) == TIME_MAP_ROLE] warnings = [TWO_CLAIM] if len(claiming) > 1 else [] @@ -83,7 +83,7 @@ def find_time_map( timeline_id=timeline.id, guessed=guessed, reason=reason, - warnings=warnings, + warnings=tuple(warnings), ) diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 00d162b99..4606871cd 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -50,7 +50,7 @@ def test_the_timeline_with_the_role_is_the_time_map(): assert found.time_map is not None assert found.time_map.timeline_id == second.id assert not found.guessed and not found.time_map.guessed - assert found.reason is None and found.warnings == [] + assert found.reason is None and found.warnings == () def test_without_the_role_the_first_beat_timeline_by_ordinal_is_guessed(): @@ -64,7 +64,7 @@ def test_without_the_role_the_first_beat_timeline_by_ordinal_is_guessed(): assert found.timeline_id == earlier.id assert found.time_map.timeline_id == earlier.id assert found.guessed and found.time_map.guessed - assert found.reason is None and found.warnings == [] + assert found.reason is None and found.warnings == () @pytest.mark.parametrize("claimants", [2, 3]) @@ -78,7 +78,7 @@ def test_when_several_claim_the_role_the_first_by_ordinal_wins(claimants): assert found.timeline_id == claiming[-1].id assert found.time_map.timeline_id == claiming[-1].id assert not found.guessed - assert found.warnings == [TWO_CLAIM] + assert found.warnings == (TWO_CLAIM,) def test_the_callers_roles_count_not_only_the_files(): @@ -90,7 +90,7 @@ def test_the_callers_roles_count_not_only_the_files(): found = find_time_map(doc, roles) assert found.timeline_id == in_manifest.id and not found.guessed - assert found.warnings == [] + assert found.warnings == () assert time_map_of(doc, roles).timeline_id == in_manifest.id assert time_map_of(doc).timeline_id == in_file.id @@ -161,7 +161,7 @@ def test_no_other_timeline_stands_in_for_one_without_a_time_map(make, reason, ro assert found.timeline_id == chosen.id assert found.guessed is (role is None) assert found.reason == reason - assert found.warnings == [] + assert found.warnings == () assert time_map_of(doc) is None @@ -170,17 +170,19 @@ def test_a_document_without_beat_timelines_has_no_time_map(caplog): found = find_time_map(doc) assert found == TimeMapFound( - time_map=None, timeline_id=None, guessed=False, reason=None, warnings=[] + time_map=None, timeline_id=None, guessed=False, reason=None, warnings=() ) assert time_map_of(doc) is None assert not caplog.records -def test_found_is_frozen(): - found = find_time_map(document(_beats(1))) +def test_found_is_frozen_and_hashes(): + found = find_time_map(document(*(_beats(j, role="time_map") for j in (1, 2)))) with pytest.raises(dataclasses.FrozenInstanceError): found.guessed = False # type: ignore[misc] + assert isinstance(found.warnings, tuple) + assert hash(found) == hash(found) @pytest.mark.parametrize("media_length", [19.5, 20.0]) @@ -230,7 +232,7 @@ def test_no_media_warning_when_the_media_lasts_past_the_last_beat( with caplog.at_level(logging.WARNING): found = find_time_map(doc) - assert found.time_map is not None and found.warnings == [] + assert found.time_map is not None and found.warnings == () assert not caplog.records @@ -241,7 +243,7 @@ def test_no_media_warning_without_a_time_map(caplog): with caplog.at_level(logging.WARNING): found = find_time_map(doc) - assert found.time_map is None and found.warnings == [] + assert found.time_map is None and found.warnings == () assert not caplog.records From 1452f3dbdb1b30421dc18146a027da26d45e7be4 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:28:02 +0200 Subject: [PATCH 03/27] fix(core): one test for whether the media ends by the last beat The closing barline and its warning each made the comparison, one as the other's negation, so a NaN length gave a warning that the barline ignored. Both call _ends_by now. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/timemap/table.py | 10 ++++++++-- .../tilia-core/tests/timemap/test_timemap_choose.py | 11 +++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/table.py b/packages/tilia-core/src/tilia_core/timemap/table.py index 5cb9e0b66..6fcf3eb1f 100644 --- a/packages/tilia-core/src/tilia_core/timemap/table.py +++ b/packages/tilia-core/src/tilia_core/timemap/table.py @@ -232,7 +232,7 @@ def _closing_barline(document: tla.Document, rows: Sequence[RowIn]) -> float: *before, last = [t for row in rows[-2:] for t in row.beat_times][-2:] end = last + (last - before[0] if before else 0.0) media_length = _media_length(document) - if media_length is None or media_length <= last: + if media_length is None or _ends_by(media_length, last): return end # past the media, the gap alone; find_time_map warns (past_media) return min(end, media_length) @@ -241,7 +241,7 @@ def past_media(document: tla.Document, last_beat: float) -> str | None: """The warning that the last beat is at or after the end of the media, so that the last measure ends one gap after it; None when it isn't.""" media_length = _media_length(document) - if media_length is None or media_length > last_beat: + if media_length is None or not _ends_by(media_length, last_beat): return None return ( f"the last beat ({last_beat}) is at or after the end of the media " @@ -254,6 +254,12 @@ def _media_length(document: tla.Document) -> float | None: return document.media_length if document.time_unit == "seconds" else None +def _ends_by(media_length: float, last_beat: float) -> bool: + # Whether the media ends at or before the last beat: one test for the + # closing barline and its warning, so they can't disagree. + return media_length <= last_beat + + def find_rows( rows: Sequence[MeasureRow], number: int | None = None, diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 4606871cd..9d5cebad6 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -236,6 +236,17 @@ def test_no_media_warning_when_the_media_lasts_past_the_last_beat( assert not caplog.records +def test_no_media_warning_for_a_media_length_that_isnt_a_number(caplog): + # Like the closing barline, which then keeps the gap: no length, no warning. + doc = document(pickup_piece(), media_length=float("nan")) + + with caplog.at_level(logging.WARNING): + found = find_time_map(doc) + + assert found.time_map.end == 21.0 and found.warnings == () + assert not caplog.records + + def test_no_media_warning_without_a_time_map(caplog): # The table is folded, so it has no closing barline to warn about. doc = document(_folded(), media_length=2.0) From 8f357bc90de51a75c8e7e0d6ffc3644cd0139d7d Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:28:12 +0200 Subject: [PATCH 04/27] refactor(core): one lookup of a beat timeline for time_map_for and measure_rows Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/__init__.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/__init__.py b/packages/tilia-core/src/tilia_core/timemap/__init__.py index d0229ab00..57f2c1468 100644 --- a/packages/tilia-core/src/tilia_core/timemap/__init__.py +++ b/packages/tilia-core/src/tilia_core/timemap/__init__.py @@ -102,8 +102,8 @@ def time_map_of( def time_map_for(document: tla.Document, timeline_id: str) -> TimeMap | None: """The time map of the beat timeline `timeline_id`; None when it has none.""" - timeline = document.timelines.get(timeline_id) - if timeline is None or timeline.kind != "beat": + timeline = _beat_timeline(document, timeline_id) + if timeline is None: return None time_map, _ = build_time_map(document, timeline) return time_map @@ -113,10 +113,15 @@ def measure_rows(document: tla.Document, timeline_id: str) -> list[MeasureRow]: """The measure table of the beat timeline `timeline_id`, in its order. Also for a folded table, which has no time map. No rows while a beat has no - usable time or the table is broken, and none for a timeline that isn't a - beat timeline. + usable time or a row's beats don't start at its downbeat (`build_rows`), + and none for a timeline that isn't a beat timeline. """ - timeline = document.timelines.get(timeline_id) - if timeline is None or timeline.kind != "beat": + timeline = _beat_timeline(document, timeline_id) + if timeline is None: return [] return build_rows(document, timeline) + + +def _beat_timeline(document: tla.Document, timeline_id: str) -> tla.Timeline | None: + timeline = document.timelines.get(timeline_id) + return timeline if timeline is not None and timeline.kind == "beat" else None From ba8ade452b83b4c3a1a579d9abd6c2dc1aa11b92 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:28:22 +0200 Subject: [PATCH 05/27] test(core): say what the media warning test checks The warning is returned and logged once for each call of find_time_map, which the index makes once when it reads a file. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/tests/timemap/test_timemap_choose.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 9d5cebad6..4467cff89 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -186,10 +186,11 @@ def test_found_is_frozen_and_hashes(): @pytest.mark.parametrize("media_length", [19.5, 20.0]) -def test_the_last_beat_at_or_after_the_end_of_the_media_is_warned_once( +def test_the_last_beat_at_or_after_the_end_of_the_media_is_returned_and_logged( media_length, caplog ): - # FR-082: returned, and logged with the timeline's id, once per file. + # FR-082: returned, and logged once with the timeline's id. The index calls + # find_time_map once when it reads a file. timeline = pickup_piece() # the last beat at 20 s doc = document(timeline, media_length=media_length) From b410c0e29d645f51c25e72fa72cd404359c55bd5 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:28:33 +0200 Subject: [PATCH 06/27] fix(core): give no rows for a table out of time order A table whose beats are out of time order, or has two at the same time, gives no rows, as for a beat without a time, so no measure ends before it starts. build_rows and build_time_map share the check (disorder), and build_time_map's reasons don't change. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/table.py | 32 ++++++++++++++++--- .../src/tilia_core/timemap/timemap.py | 9 +++--- .../tests/timemap/test_timemap_choose.py | 15 +++++++++ 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/table.py b/packages/tilia-core/src/tilia_core/timemap/table.py index 6fcf3eb1f..c24a38dbc 100644 --- a/packages/tilia-core/src/tilia_core/timemap/table.py +++ b/packages/tilia-core/src/tilia_core/timemap/table.py @@ -3,6 +3,7 @@ from __future__ import annotations +import itertools import math from collections.abc import Iterable, Mapping, Sequence from dataclasses import dataclass, field @@ -14,6 +15,8 @@ from tilia_core.timemap.meters import meters_of from tilia_core.timemap.units import MeasureMeter, format_units +SAME_TIME = 1e-6 # two beats closer than this are at the same time + class FrozenMetadata(dict[str, "str | tuple[str, ...]"]): """A measure's metadata, read-only, with its lists as tuples. @@ -104,15 +107,36 @@ def __post_init__(self) -> None: def build_rows(document: tla.Document, timeline: tla.Timeline) -> list[MeasureRow]: """The timeline's measure table, in its order; also for a folded table. - None while a beat has no usable time (`missing_time` says which), or while a - row's beats don't start at its downbeat (`broken_row`): rows built without - them would be wrong. + None while a beat has no usable time (`missing_time` says which), while a + row's beats don't start at its downbeat (`broken_row`), or while two beats + are at the same time or out of time order (`disorder`): rows built then + would be wrong, with measures that end before they start. """ - if missing_time(timeline) is not None or broken_row(timeline) is not None: + if _usable_times(timeline) is None: return [] return build_rows_from(document, timeline, rows_of(timeline)) +def _usable_times(timeline: tla.Timeline) -> list[float] | None: + """The times of the table's beats, in its order; None when it can have no rows.""" + if missing_time(timeline) is not None or broken_row(timeline) is not None: + return None + rows = timeline.measures.rows if timeline.measures is not None else [] + times = [timeline.components[b].attrs["time"] for row in rows for b in row.beats] + return None if disorder(times) is not None else times + + +def disorder(times: Sequence[float]) -> str | None: + """The first two beats at the same time, or the first out of time order, as + a reason; else None.""" + for a, b in itertools.pairwise(times): + if abs(b - a) < SAME_TIME: + return f"two beats at the same time ({a})" + if b < a: + return f"beats out of time order (at {b})" + return None + + def missing_time(timeline: tla.Timeline) -> str | None: """The first beat without a usable time, as a reason naming its measure; else None. diff --git a/packages/tilia-core/src/tilia_core/timemap/timemap.py b/packages/tilia-core/src/tilia_core/timemap/timemap.py index 1fe0f77ab..2c7e439a3 100644 --- a/packages/tilia-core/src/tilia_core/timemap/timemap.py +++ b/packages/tilia-core/src/tilia_core/timemap/timemap.py @@ -19,6 +19,7 @@ MeasureRow, broken_row, build_rows_from, + disorder, is_folded, missing_time, past_media, @@ -193,11 +194,9 @@ def build_time_map( times = [t for row in rows_in for t in row.beat_times] if len(times) < 2: return None, "fewer than two beats" - for a, b in itertools.pairwise(times): - if abs(b - a) < EPS: - return None, f"two beats at the same time ({a})" - if b < a: - return None, f"beats out of time order (at {b})" + reason = disorder(times) + if reason is not None: + return None, reason if is_folded(rows_in): return None, FOLDED rows = build_rows_from(document, timeline, rows_in) diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 4467cff89..16bc80578 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -294,6 +294,21 @@ def test_measure_rows_give_none_while_a_beat_has_no_usable_time(): assert measure_rows(document(timeline), timeline.id) == [] +def _out_of_order(**kwargs) -> tla.Timeline: + return beat_timeline( + [{"time": 0.0, "measure": {}}, {"time": 2.0, "measure": {}}, {"time": 1.0}], + **kwargs, + ) + + +@pytest.mark.parametrize("make", [_same_time, _out_of_order]) +def test_measure_rows_give_none_while_beats_are_out_of_order_or_share_a_time(make): + # Anne, 2026-10-14: no measure that ends before it starts. + timeline = make() + + assert measure_rows(document(timeline), timeline.id) == [] + + def test_measure_rows_of_a_timeline_that_is_not_a_beat_timeline(): # Even one that carries a table, as only a beat timeline should. beats, markers = _beats(1), _other("marker", 2) From 9f6ea730cebe4a03f178a55e231fe9e4209b8a94 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:28:43 +0200 Subject: [PATCH 07/27] feat(core): warn about every beat timeline past the media find_time_map warns about every beat timeline whose last beat is at or after the end of the media, the time map's or not, folded tables too, once for each call; each warning names its timeline. A table without rows gets none. table.media_warning reads the table, so TimeMap's private beats are no longer needed. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/__init__.py | 22 ++++----- .../src/tilia_core/timemap/table.py | 10 +++++ .../src/tilia_core/timemap/timemap.py | 7 --- .../tests/timemap/test_timemap_choose.py | 45 ++++++++++++++----- 4 files changed, 53 insertions(+), 31 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/__init__.py b/packages/tilia-core/src/tilia_core/timemap/__init__.py index 57f2c1468..a0e41ff95 100644 --- a/packages/tilia-core/src/tilia_core/timemap/__init__.py +++ b/packages/tilia-core/src/tilia_core/timemap/__init__.py @@ -7,13 +7,8 @@ from dataclasses import dataclass from tilia_core import tla -from tilia_core.timemap.table import MeasureRow, build_rows -from tilia_core.timemap.timemap import ( - Positions, - TimeMap, - build_time_map, - media_warning, -) +from tilia_core.timemap.table import MeasureRow, build_rows, media_warning +from tilia_core.timemap.timemap import Positions, TimeMap, build_time_map __all__ = [ "MeasureRow", @@ -40,7 +35,7 @@ class TimeMapFound: timeline_id: str | None # the chosen beat timeline, even when it gives no time map guessed: bool # no timeline had the role time_map: the first beat timeline was taken reason: str | None # why the chosen timeline gives no time map - warnings: tuple[str, ...] # several claim the role; the last beat past the media + warnings: tuple[str, ...] # several claim the role; a last beat past the media def find_time_map( @@ -57,8 +52,9 @@ def find_time_map( has no bars, and `reason` says why. A document without a beat timeline has no time map, and nothing is wrong with it: no reason, no warning. - The warning that the last beat is at or after the end of the media is - logged here, once for each call, and not when a table is built. + Every beat timeline whose last beat is at or after the end of the media + gets a warning, with its id, the time map's or not: it is logged here, once + for each call, and not when a table is built. """ beat_timelines = sorted( (t for t in document.timelines.values() if t.kind == "beat"), @@ -73,10 +69,10 @@ def find_time_map( timeline = (claiming or beat_timelines)[0] guessed = not claiming time_map, reason = build_time_map(document, timeline, guessed=guessed) - if time_map is not None: - warning = media_warning(document, time_map) + for beats in beat_timelines: + warning = media_warning(document, beats) if warning is not None: - logger.warning("Timeline %s: %s.", timeline.id, warning) + logger.warning("%s.", warning) warnings.append(warning) return TimeMapFound( time_map=time_map, diff --git a/packages/tilia-core/src/tilia_core/timemap/table.py b/packages/tilia-core/src/tilia_core/timemap/table.py index c24a38dbc..c173356f4 100644 --- a/packages/tilia-core/src/tilia_core/timemap/table.py +++ b/packages/tilia-core/src/tilia_core/timemap/table.py @@ -137,6 +137,16 @@ def disorder(times: Sequence[float]) -> str | None: return None +def media_warning(document: tla.Document, timeline: tla.Timeline) -> str | None: + """The warning, with the timeline's id, that its last beat is at or after + the end of the media; None when it isn't, or when the table has no rows.""" + times = _usable_times(timeline) + if not times: + return None + warning = past_media(document, times[-1]) + return None if warning is None else f"Timeline {timeline.id}: {warning}" + + def missing_time(timeline: tla.Timeline) -> str | None: """The first beat without a usable time, as a reason naming its measure; else None. diff --git a/packages/tilia-core/src/tilia_core/timemap/timemap.py b/packages/tilia-core/src/tilia_core/timemap/timemap.py index 2c7e439a3..e96319fde 100644 --- a/packages/tilia-core/src/tilia_core/timemap/timemap.py +++ b/packages/tilia-core/src/tilia_core/timemap/timemap.py @@ -22,7 +22,6 @@ disorder, is_folded, missing_time, - past_media, ) TOLERANCE = 0.1 # in the file's unit: a time this close to a beat is on it @@ -207,9 +206,3 @@ def build_time_map( _times=tuple(times), ) return time_map, None - - -def media_warning(document: tla.Document, time_map: TimeMap) -> str | None: - """The warning that the map's last beat is at or after the end of the - media, which leaves the closing barline one gap after it; else None.""" - return past_media(document, time_map._times[-1]) diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 16bc80578..7d1fbf721 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -200,8 +200,8 @@ def test_the_last_beat_at_or_after_the_end_of_the_media_is_returned_and_logged( assert found.time_map.end == 21.0 (warning,) = found.warnings assert warning == ( - f"the last beat (20.0) is at or after the end of the media ({media_length}); " - "the last measure ends one gap after it" + f"Timeline {timeline.id}: the last beat (20.0) is at or after the end of " + f"the media ({media_length}); the last measure ends one gap after it" ) (record,) = caplog.records assert record.name == "tilia_core.timemap" @@ -211,13 +211,33 @@ def test_the_last_beat_at_or_after_the_end_of_the_media_is_returned_and_logged( def test_the_warnings_come_in_the_order_they_were_found(): - claiming = [_beats(j, role="time_map") for j in (1, 2)] + # The claim first, then each timeline past the media, by ordinal. + later, earlier = _beats(2, role="time_map"), _beats(1, role="time_map") - found = find_time_map(document(*claiming, media_length=2.0)) + found = find_time_map(document(later, earlier, media_length=2.0)) - assert len(found.warnings) == 2 + assert len(found.warnings) == 3 assert found.warnings[0] == TWO_CLAIM - assert found.warnings[1].startswith("the last beat (3.0)") + assert found.warnings[1].startswith(f"Timeline {earlier.id}: the last beat (3.0)") + assert found.warnings[2].startswith(f"Timeline {later.id}: the last beat (3.0)") + + +def test_every_beat_timeline_past_the_media_is_warned_about(caplog): + # Anne, 2026-10-14: the tables a score or an import names too, once for + # each call of find_time_map. A table without rows has no closing barline. + default, named = _beats(1, role="time_map"), _beats(2, last=7) + timeless = _beats(3, last=7) + timeless.components[timeless.measures.rows[0].beats[1]].attrs["time"] = None + doc = document(default, named, timeless, media_length=5.0) + + with caplog.at_level(logging.WARNING): + found = find_time_map(doc) + + assert found.timeline_id == default.id + (warning,) = found.warnings + assert warning.startswith(f"Timeline {named.id}: the last beat (7.0)") + (record,) = caplog.records + assert record.getMessage() == f"{warning}." @pytest.mark.parametrize( @@ -248,15 +268,18 @@ def test_no_media_warning_for_a_media_length_that_isnt_a_number(caplog): assert not caplog.records -def test_no_media_warning_without_a_time_map(caplog): - # The table is folded, so it has no closing barline to warn about. - doc = document(_folded(), media_length=2.0) +def test_a_folded_table_past_the_media_is_warned_about(caplog): + # It has no time map, but its rows have a closing barline (FR-013). + folded = _folded() + doc = document(folded, media_length=2.0) with caplog.at_level(logging.WARNING): found = find_time_map(doc) - assert found.time_map is None and found.warnings == () - assert not caplog.records + assert found.time_map is None and found.reason == FOLDED + (warning,) = found.warnings + assert warning.startswith(f"Timeline {folded.id}: the last beat (3.0)") + assert len(caplog.records) == 1 def test_tables_and_named_time_maps_log_nothing_about_the_media(caplog): From f2e1ddf15aaf3a21685167fbf6b8b6423d3dae53 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:28:54 +0200 Subject: [PATCH 08/27] feat(core): let a role that is a list claim time_map Metadata values may be lists: a role that is one claims time_map when it holds it. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/__init__.py | 24 +++++++++++-------- .../tests/timemap/test_timemap_choose.py | 15 ++++++++++++ 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/__init__.py b/packages/tilia-core/src/tilia_core/timemap/__init__.py index a0e41ff95..106e08f77 100644 --- a/packages/tilia-core/src/tilia_core/timemap/__init__.py +++ b/packages/tilia-core/src/tilia_core/timemap/__init__.py @@ -46,11 +46,12 @@ def find_time_map( `roles` gives each timeline's role by its id, as the caller resolved them (the index: from the file and the corpus's manifest), and a timeline it - leaves out has none; without it, the roles are the file's. When several - beat timelines have the role, the first by ordinal wins, with a warning. - When the chosen timeline gives no time map, no other stands in: the file - has no bars, and `reason` says why. A document without a beat timeline has - no time map, and nothing is wrong with it: no reason, no warning. + leaves out has none; without it, the roles are the file's. A role that is a + list claims time_map when it holds it. When several beat timelines have the + role, the first by ordinal wins, with a warning. When the chosen timeline + gives no time map, no other stands in: the file has no bars, and `reason` + says why. A document without a beat timeline has no time map, and nothing + is wrong with it: no reason, no warning. Every beat timeline whose last beat is at or after the end of the media gets a warning, with its id, the time map's or not: it is logged here, once @@ -64,7 +65,7 @@ def find_time_map( return TimeMapFound( time_map=None, timeline_id=None, guessed=False, reason=None, warnings=() ) - claiming = [t for t in beat_timelines if _role(t, roles) == TIME_MAP_ROLE] + claiming = [t for t in beat_timelines if _claims_the_role(t, roles)] warnings = [TWO_CLAIM] if len(claiming) > 1 else [] timeline = (claiming or beat_timelines)[0] guessed = not claiming @@ -83,10 +84,13 @@ def find_time_map( ) -def _role(timeline: tla.Timeline, roles: Mapping[str, str | None] | None) -> object: - if roles is None: - return timeline.metadata.get("role") - return roles.get(timeline.id) +def _claims_the_role( + timeline: tla.Timeline, roles: Mapping[str, str | None] | None +) -> bool: + role = timeline.metadata.get("role") if roles is None else roles.get(timeline.id) + if isinstance(role, (list, tuple)): + return TIME_MAP_ROLE in role + return role == TIME_MAP_ROLE def time_map_of( diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 7d1fbf721..828643ff6 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -116,6 +116,21 @@ def test_only_a_beat_timeline_can_have_the_role(): assert found.timeline_id == beats.id and found.guessed +@pytest.mark.parametrize( + "role, claims", + [(["time_map"], True), (["analysis", "time_map"], True), (["analysis"], False)], +) +def test_a_role_list_that_holds_time_map_claims_it(role, claims): + # Anne, 2026-10-14: metadata values may be lists. + first, listed = _beats(1), _beats(2) + listed.metadata["role"] = role + + found = find_time_map(document(first, listed)) + + assert found.timeline_id == (listed.id if claims else first.id) + assert found.guessed is not claims + + def test_a_timeline_that_names_a_beat_timeline_gets_its_table_and_time_map(): # Spec, US4 scenario 5: a score timeline names its table (WP3's FR-133). default, named = _beats(1, role="time_map"), _beats(2, last=7) From b7db78ea7962bfa6f188dc553606fccb0d58837a Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:29:04 +0200 Subject: [PATCH 09/27] fix(core): sort an ordinal that isn't a whole number after the others The reader doesn't refuse one yet, so a file with one got a TypeError instead of a time map. A whole number written as a float sorts by its value. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/__init__.py | 13 ++++++++++++- .../tests/timemap/test_timemap_choose.py | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/__init__.py b/packages/tilia-core/src/tilia_core/timemap/__init__.py index 106e08f77..3ba948162 100644 --- a/packages/tilia-core/src/tilia_core/timemap/__init__.py +++ b/packages/tilia-core/src/tilia_core/timemap/__init__.py @@ -59,7 +59,7 @@ def find_time_map( """ beat_timelines = sorted( (t for t in document.timelines.values() if t.kind == "beat"), - key=lambda t: t.ordinal, + key=_by_ordinal, ) if not beat_timelines: return TimeMapFound( @@ -84,6 +84,17 @@ def find_time_map( ) +def _by_ordinal(timeline: tla.Timeline) -> tuple[int, float]: + # An ordinal that isn't a whole number, which the reader doesn't refuse + # yet, comes after the others, in the document's order. + ordinal: object = timeline.ordinal + if isinstance(ordinal, int) and not isinstance(ordinal, bool): + return 0, ordinal + if isinstance(ordinal, float) and ordinal.is_integer(): + return 0, ordinal + return 1, 0 + + def _claims_the_role( timeline: tla.Timeline, roles: Mapping[str, str | None] | None ) -> bool: diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 828643ff6..985f2ed5e 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -131,6 +131,24 @@ def test_a_role_list_that_holds_time_map_claims_it(role, claims): assert found.guessed is not claims +@pytest.mark.parametrize("ordinal", [None, "1", 1.5, float("nan"), True, [1]]) +def test_an_ordinal_that_isnt_a_whole_number_sorts_after_the_others(ordinal): + # Anne, 2026-10-14: nothing raises while the reader lets one through (#7). + odd, other = _beats(1), _beats(3) + odd.ordinal = ordinal + + found = find_time_map(document(odd, other)) + + assert found.timeline_id == other.id and found.guessed + + +def test_a_whole_ordinal_written_as_a_float_sorts_by_its_value(): + two, three = _beats(1), _beats(3) + two.ordinal = 2.0 + + assert find_time_map(document(three, two)).timeline_id == two.id + + def test_a_timeline_that_names_a_beat_timeline_gets_its_table_and_time_map(): # Spec, US4 scenario 5: a score timeline names its table (WP3's FR-133). default, named = _beats(1, role="time_map"), _beats(2, last=7) From 7e0b5bd2e9594d3d33cf30c91344efa25e88f7ba Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:18:11 +0200 Subject: [PATCH 10/27] refactor(core): one chain of table checks (table_problem) table_problem gives the reason a table has no rows: a beat without a usable time, a row whose beats don't start at its downbeat, or two beats at the same time or out of time order. build_rows, media_warning and build_time_map share it, so they can't drift apart; the docstrings of build_rows and measure_rows name it. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/__init__.py | 7 ++-- .../src/tilia_core/timemap/table.py | 32 ++++++++++++------- .../src/tilia_core/timemap/timemap.py | 9 ++---- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/__init__.py b/packages/tilia-core/src/tilia_core/timemap/__init__.py index 3ba948162..4a80e97b3 100644 --- a/packages/tilia-core/src/tilia_core/timemap/__init__.py +++ b/packages/tilia-core/src/tilia_core/timemap/__init__.py @@ -123,9 +123,10 @@ def time_map_for(document: tla.Document, timeline_id: str) -> TimeMap | None: def measure_rows(document: tla.Document, timeline_id: str) -> list[MeasureRow]: """The measure table of the beat timeline `timeline_id`, in its order. - Also for a folded table, which has no time map. No rows while a beat has no - usable time or a row's beats don't start at its downbeat (`build_rows`), - and none for a timeline that isn't a beat timeline. + Also for a folded table, which has no time map. No rows while the table + has a problem (`table.table_problem`: a beat without a usable time, a row + whose beats don't start at its downbeat, or two beats at the same time or + out of time order), and none for a timeline that isn't a beat timeline. """ timeline = _beat_timeline(document, timeline_id) if timeline is None: diff --git a/packages/tilia-core/src/tilia_core/timemap/table.py b/packages/tilia-core/src/tilia_core/timemap/table.py index c173356f4..cb106cf07 100644 --- a/packages/tilia-core/src/tilia_core/timemap/table.py +++ b/packages/tilia-core/src/tilia_core/timemap/table.py @@ -107,23 +107,29 @@ def __post_init__(self) -> None: def build_rows(document: tla.Document, timeline: tla.Timeline) -> list[MeasureRow]: """The timeline's measure table, in its order; also for a folded table. - None while a beat has no usable time (`missing_time` says which), while a - row's beats don't start at its downbeat (`broken_row`), or while two beats - are at the same time or out of time order (`disorder`): rows built then - would be wrong, with measures that end before they start. + No rows while the table has a problem (`table_problem` says which): rows + built then would be wrong, with measures that end before they start. """ - if _usable_times(timeline) is None: + if table_problem(timeline) is not None: return [] return build_rows_from(document, timeline, rows_of(timeline)) -def _usable_times(timeline: tla.Timeline) -> list[float] | None: - """The times of the table's beats, in its order; None when it can have no rows.""" - if missing_time(timeline) is not None or broken_row(timeline) is not None: - return None +def table_problem(timeline: tla.Timeline) -> str | None: + """Why the timeline's table can have no rows, as a reason; else None. + + A beat without a usable time (`missing_time`), a row whose beats don't + start at its downbeat (`broken_row`), or two beats at the same time or out + of time order (`disorder`). + """ + return missing_time(timeline) or broken_row(timeline) or disorder(_times(timeline)) + + +def _times(timeline: tla.Timeline) -> list[float]: + # The times of the table's beats, in its order, once missing_time has + # found each of them usable. rows = timeline.measures.rows if timeline.measures is not None else [] - times = [timeline.components[b].attrs["time"] for row in rows for b in row.beats] - return None if disorder(times) is not None else times + return [timeline.components[b].attrs["time"] for row in rows for b in row.beats] def disorder(times: Sequence[float]) -> str | None: @@ -140,7 +146,9 @@ def disorder(times: Sequence[float]) -> str | None: def media_warning(document: tla.Document, timeline: tla.Timeline) -> str | None: """The warning, with the timeline's id, that its last beat is at or after the end of the media; None when it isn't, or when the table has no rows.""" - times = _usable_times(timeline) + if table_problem(timeline) is not None: + return None + times = _times(timeline) if not times: return None warning = past_media(document, times[-1]) diff --git a/packages/tilia-core/src/tilia_core/timemap/timemap.py b/packages/tilia-core/src/tilia_core/timemap/timemap.py index e96319fde..40ebc940e 100644 --- a/packages/tilia-core/src/tilia_core/timemap/timemap.py +++ b/packages/tilia-core/src/tilia_core/timemap/timemap.py @@ -17,11 +17,9 @@ from tilia_core.timemap._rows import rows_of from tilia_core.timemap.table import ( MeasureRow, - broken_row, build_rows_from, - disorder, is_folded, - missing_time, + table_problem, ) TOLERANCE = 0.1 # in the file's unit: a time this close to a beat is on it @@ -186,16 +184,13 @@ def build_time_map( table = timeline.measures if table is None or (not table.rows and timeline.components): return None, "no measure table" - reason = missing_time(timeline) or broken_row(timeline) + reason = table_problem(timeline) if reason is not None: return None, reason rows_in = rows_of(timeline) times = [t for row in rows_in for t in row.beat_times] if len(times) < 2: return None, "fewer than two beats" - reason = disorder(times) - if reason is not None: - return None, reason if is_folded(rows_in): return None, FOLDED rows = build_rows_from(document, timeline, rows_in) From 5ac2616a47ec9c4ffc60f6953fda5cbf7c228782 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:18:20 +0200 Subject: [PATCH 11/27] refactor(core): one EPS for snapping and the same-time check EPS moves to table.py, which timemap.py imports, in place of a second constant with the same value. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/timemap/table.py | 6 ++++-- packages/tilia-core/src/tilia_core/timemap/timemap.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/table.py b/packages/tilia-core/src/tilia_core/timemap/table.py index cb106cf07..9b4201845 100644 --- a/packages/tilia-core/src/tilia_core/timemap/table.py +++ b/packages/tilia-core/src/tilia_core/timemap/table.py @@ -15,7 +15,9 @@ from tilia_core.timemap.meters import meters_of from tilia_core.timemap.units import MeasureMeter, format_units -SAME_TIME = 1e-6 # two beats closer than this are at the same time +# Float noise: two beats closer than this are at the same time, and a time this +# much beyond the tolerance is still on a beat. +EPS = 1e-6 class FrozenMetadata(dict[str, "str | tuple[str, ...]"]): @@ -136,7 +138,7 @@ def disorder(times: Sequence[float]) -> str | None: """The first two beats at the same time, or the first out of time order, as a reason; else None.""" for a, b in itertools.pairwise(times): - if abs(b - a) < SAME_TIME: + if abs(b - a) < EPS: return f"two beats at the same time ({a})" if b < a: return f"beats out of time order (at {b})" diff --git a/packages/tilia-core/src/tilia_core/timemap/timemap.py b/packages/tilia-core/src/tilia_core/timemap/timemap.py index 40ebc940e..5e486d5c1 100644 --- a/packages/tilia-core/src/tilia_core/timemap/timemap.py +++ b/packages/tilia-core/src/tilia_core/timemap/timemap.py @@ -16,6 +16,7 @@ from tilia_core import tla from tilia_core.timemap._rows import rows_of from tilia_core.timemap.table import ( + EPS, MeasureRow, build_rows_from, is_folded, @@ -23,7 +24,6 @@ ) TOLERANCE = 0.1 # in the file's unit: a time this close to a beat is on it -EPS = 1e-6 # added to the tolerance, so float noise never decides whether a time is on a beat FOLDED = ( "the table lists the measures played next (`next`), " From 46b9d081f49ccc278fa5b9f11f69bea923105563 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:18:30 +0200 Subject: [PATCH 12/27] test(core): check that TimeMapFound hashes by hashing it hash(found) == hash(found) couldn't fail once hash returned. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/tests/timemap/test_timemap_choose.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 985f2ed5e..41fb792a4 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -215,7 +215,7 @@ def test_found_is_frozen_and_hashes(): with pytest.raises(dataclasses.FrozenInstanceError): found.guessed = False # type: ignore[misc] assert isinstance(found.warnings, tuple) - assert hash(found) == hash(found) + hash(found) @pytest.mark.parametrize("media_length", [19.5, 20.0]) From bcdacdbe22671f2b7f9b9da822869298a64d10b4 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:18:39 +0200 Subject: [PATCH 13/27] test(core): a table out of time order gets no media warning It has no rows, so no closing barline. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../tilia-core/tests/timemap/test_timemap_choose.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 41fb792a4..34d44ac0c 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -301,6 +301,19 @@ def test_no_media_warning_for_a_media_length_that_isnt_a_number(caplog): assert not caplog.records +@pytest.mark.parametrize("make", ["_same_time", "_out_of_order"]) +def test_a_table_out_of_time_order_gets_no_media_warning(make, caplog): + # It has no rows, so no closing barline. + timeline = globals()[make]() + doc = document(timeline, media_length=0.5) + + with caplog.at_level(logging.WARNING): + found = find_time_map(doc) + + assert found.time_map is None and found.warnings == () + assert not caplog.records + + def test_a_folded_table_past_the_media_is_warned_about(caplog): # It has no time map, but its rows have a closing barline (FR-013). folded = _folded() From b670fc8b52b8297beaef7bc96ebf32cb6608210a Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Thu, 15 Oct 2026 12:04:40 +0200 Subject: [PATCH 14/27] fix(core): treat a media length the format doesn't allow as no length The reader keeps media.length as it finds it. Text made find_time_map raise TypeError, and true counted as one second. The format allows a number above zero or null, so anything else (text, a bool, NaN, infinity, zero or less) now caps nothing and warns about nothing. A NaN closing barline no longer depends on min's argument order. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../tilia-core/src/tilia_core/timemap/table.py | 14 ++++++++++++-- .../tests/timemap/test_timemap_choose.py | 15 +++++++++++---- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/table.py b/packages/tilia-core/src/tilia_core/timemap/table.py index 9b4201845..697d2672e 100644 --- a/packages/tilia-core/src/tilia_core/timemap/table.py +++ b/packages/tilia-core/src/tilia_core/timemap/table.py @@ -294,8 +294,18 @@ def past_media(document: tla.Document, last_beat: float) -> str | None: def _media_length(document: tla.Document) -> float | None: - # The media's length is in seconds: it doesn't cap a quarters file. - return document.media_length if document.time_unit == "seconds" else None + """The media's length, when it caps the closing barline; else None. + + It is in seconds, so it doesn't cap a quarters file. The format allows a + number above zero, or none; the reader doesn't check, so anything else + (text, a bool, NaN, infinity, zero or less) counts as none. + """ + length: object = document.media_length + if document.time_unit != "seconds" or isinstance(length, bool): + return None + if not isinstance(length, (int, float)) or not math.isfinite(length): + return None + return length if length > 0 else None def _ends_by(media_length: float, last_beat: float) -> bool: diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 34d44ac0c..8227c3cf3 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -290,14 +290,21 @@ def test_no_media_warning_when_the_media_lasts_past_the_last_beat( assert not caplog.records -def test_no_media_warning_for_a_media_length_that_isnt_a_number(caplog): - # Like the closing barline, which then keeps the gap: no length, no warning. - doc = document(pickup_piece(), media_length=float("nan")) +@pytest.mark.parametrize( + "media_length", ["20", True, float("nan"), float("inf"), 0, 0.0, -5.0, [19.5]] +) +def test_a_media_length_the_format_doesnt_allow_is_no_length(media_length, caplog): + # The schema allows a number above zero, or null; the reader doesn't check. + # No cap on the closing barline, no warning, and nothing raises. + timeline = pickup_piece() + doc = document(timeline, media_length=media_length) with caplog.at_level(logging.WARNING): found = find_time_map(doc) + rows = measure_rows(doc, timeline.id) - assert found.time_map.end == 21.0 and found.warnings == () + assert found.time_map.end == 21.0 and rows[-1].end == 21.0 + assert found.warnings == () assert not caplog.records From 404d43c382033d7aab1b5a0e21657d49464f9dd3 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Thu, 15 Oct 2026 12:04:48 +0200 Subject: [PATCH 15/27] fix(core): leave the gap out of the media warning for a single beat A table with one beat ends its only measure on the beat, so the warning no longer says the last measure ends one gap after it. media_warning builds the text itself, and past_media goes. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/table.py | 32 +++++++++---------- .../tests/timemap/test_timemap_choose.py | 13 ++++++++ 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/table.py b/packages/tilia-core/src/tilia_core/timemap/table.py index 697d2672e..d8fd16b7b 100644 --- a/packages/tilia-core/src/tilia_core/timemap/table.py +++ b/packages/tilia-core/src/tilia_core/timemap/table.py @@ -147,14 +147,24 @@ def disorder(times: Sequence[float]) -> str | None: def media_warning(document: tla.Document, timeline: tla.Timeline) -> str | None: """The warning, with the timeline's id, that its last beat is at or after - the end of the media; None when it isn't, or when the table has no rows.""" + the end of the media; None when it isn't, or when the table has no rows. + + With more than one beat, the last measure ends one gap after the last + beat, and the warning says so; with one, it ends on the beat. + """ if table_problem(timeline) is not None: return None times = _times(timeline) - if not times: + media_length = _media_length(document) + if not times or media_length is None or not _ends_by(media_length, times[-1]): return None - warning = past_media(document, times[-1]) - return None if warning is None else f"Timeline {timeline.id}: {warning}" + warning = ( + f"Timeline {timeline.id}: the last beat ({times[-1]}) is at or after " + f"the end of the media ({media_length})" + ) + if len(times) > 1: + warning += "; the last measure ends one gap after it" + return warning def missing_time(timeline: tla.Timeline) -> str | None: @@ -277,22 +287,10 @@ def _closing_barline(document: tla.Document, rows: Sequence[RowIn]) -> float: end = last + (last - before[0] if before else 0.0) media_length = _media_length(document) if media_length is None or _ends_by(media_length, last): - return end # past the media, the gap alone; find_time_map warns (past_media) + return end # past the media, the gap alone; find_time_map warns return min(end, media_length) -def past_media(document: tla.Document, last_beat: float) -> str | None: - """The warning that the last beat is at or after the end of the media, so - that the last measure ends one gap after it; None when it isn't.""" - media_length = _media_length(document) - if media_length is None or not _ends_by(media_length, last_beat): - return None - return ( - f"the last beat ({last_beat}) is at or after the end of the media " - f"({media_length}); the last measure ends one gap after it" - ) - - def _media_length(document: tla.Document) -> float | None: """The media's length, when it caps the closing barline; else None. diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 8227c3cf3..7782cad6e 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -308,6 +308,19 @@ def test_a_media_length_the_format_doesnt_allow_is_no_length(media_length, caplo assert not caplog.records +def test_one_beat_past_the_media_is_warned_about_without_a_gap(): + # Its only measure ends on the beat: there is no gap after it. + one = beat_timeline([{"time": 5.0, "measure": {}}]) + + found = find_time_map(document(one, media_length=4.0)) + + assert found.reason == "fewer than two beats" + assert found.warnings == ( + f"Timeline {one.id}: the last beat (5.0) is at or after the end of the " + "media (4.0)", + ) + + @pytest.mark.parametrize("make", ["_same_time", "_out_of_order"]) def test_a_table_out_of_time_order_gets_no_media_warning(make, caplog): # It has no rows, so no closing barline. From 6903b609b6187861db687ddcc82d4c2297731a7c Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Thu, 15 Oct 2026 12:04:55 +0200 Subject: [PATCH 16/27] feat(core): name the timelines that claim time_map, and log the warning The warning names every claimant in ordinal order, instead of saying "two" whatever the count: "timelines A, B and C claim the role time_map; A, the first by ordinal, is used". find_time_map logs it like the media warnings, once for each call, so a caller of time_map_of doesn't lose it. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/__init__.py | 26 ++++++++++++------- .../tests/timemap/test_timemap_choose.py | 26 ++++++++++++++----- 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/__init__.py b/packages/tilia-core/src/tilia_core/timemap/__init__.py index 4a80e97b3..0b07999da 100644 --- a/packages/tilia-core/src/tilia_core/timemap/__init__.py +++ b/packages/tilia-core/src/tilia_core/timemap/__init__.py @@ -24,7 +24,6 @@ logger = logging.getLogger(__name__) TIME_MAP_ROLE = "time_map" -TWO_CLAIM = "two timelines claim the role time_map; the first by ordinal is used" @dataclass(frozen=True) @@ -48,14 +47,14 @@ def find_time_map( (the index: from the file and the corpus's manifest), and a timeline it leaves out has none; without it, the roles are the file's. A role that is a list claims time_map when it holds it. When several beat timelines have the - role, the first by ordinal wins, with a warning. When the chosen timeline - gives no time map, no other stands in: the file has no bars, and `reason` - says why. A document without a beat timeline has no time map, and nothing - is wrong with it: no reason, no warning. + role, the first by ordinal wins, with a warning that names them. When the + chosen timeline gives no time map, no other stands in: the file has no + bars, and `reason` says why. A document without a beat timeline has no + time map, and nothing is wrong with it: no reason, no warning. Every beat timeline whose last beat is at or after the end of the media - gets a warning, with its id, the time map's or not: it is logged here, once - for each call, and not when a table is built. + gets a warning too, with its id, the time map's or not. The warnings are + logged here, once for each call, and not when a table is built. """ beat_timelines = sorted( (t for t in document.timelines.values() if t.kind == "beat"), @@ -66,15 +65,16 @@ def find_time_map( time_map=None, timeline_id=None, guessed=False, reason=None, warnings=() ) claiming = [t for t in beat_timelines if _claims_the_role(t, roles)] - warnings = [TWO_CLAIM] if len(claiming) > 1 else [] + warnings = [_several_claim(claiming)] if len(claiming) > 1 else [] timeline = (claiming or beat_timelines)[0] guessed = not claiming time_map, reason = build_time_map(document, timeline, guessed=guessed) for beats in beat_timelines: warning = media_warning(document, beats) if warning is not None: - logger.warning("%s.", warning) warnings.append(warning) + for warning in warnings: + logger.warning("%s.", warning) return TimeMapFound( time_map=time_map, timeline_id=timeline.id, @@ -84,6 +84,14 @@ def find_time_map( ) +def _several_claim(claiming: list[tla.Timeline]) -> str: + ids = [timeline.id for timeline in claiming] + return ( + f"timelines {', '.join(ids[:-1])} and {ids[-1]} claim the role time_map; " + f"{ids[0]}, the first by ordinal, is used" + ) + + def _by_ordinal(timeline: tla.Timeline) -> tuple[int, float]: # An ordinal that isn't a whole number, which the reader doesn't refuse # yet, comes after the others, in the document's order. diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 7782cad6e..98cfc3777 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -15,7 +15,15 @@ ) from tilia_core.timemap.timemap import FOLDED -TWO_CLAIM = "two timelines claim the role time_map; the first by ordinal is used" + +def _claim(*claiming: tla.Timeline) -> str: + """The warning when several timelines claim the role, in ordinal order.""" + ids = [timeline.id for timeline in claiming] + named = f"{', '.join(ids[:-1])} and {ids[-1]}" + return ( + f"timelines {named} claim the role time_map; " + f"{ids[0]}, the first by ordinal, is used" + ) def _beats(ordinal: int, role: str | None = None, *, last: int = 3) -> tla.Timeline: @@ -68,17 +76,23 @@ def test_without_the_role_the_first_beat_timeline_by_ordinal_is_guessed(): @pytest.mark.parametrize("claimants", [2, 3]) -def test_when_several_claim_the_role_the_first_by_ordinal_wins(claimants): - # Spec, US4 scenario 3: one warning, however many claim it. +def test_when_several_claim_the_role_the_first_by_ordinal_wins(claimants, caplog): + # Spec, US4 scenario 3: one warning, however many claim it, naming them in + # ordinal order; returned and logged (TimeLineAnnotator/project-management#13 + # (the measure table)). unclaimed = _beats(1) claiming = [_beats(10 - j, role="time_map") for j in range(claimants)] - found = find_time_map(document(unclaimed, *claiming)) + with caplog.at_level(logging.WARNING): + found = find_time_map(document(unclaimed, *claiming)) assert found.timeline_id == claiming[-1].id assert found.time_map.timeline_id == claiming[-1].id assert not found.guessed - assert found.warnings == (TWO_CLAIM,) + assert found.warnings == (_claim(*reversed(claiming)),) + (record,) = caplog.records + assert record.name == "tilia_core.timemap" + assert record.getMessage() == f"{found.warnings[0]}." def test_the_callers_roles_count_not_only_the_files(): @@ -250,7 +264,7 @@ def test_the_warnings_come_in_the_order_they_were_found(): found = find_time_map(document(later, earlier, media_length=2.0)) assert len(found.warnings) == 3 - assert found.warnings[0] == TWO_CLAIM + assert found.warnings[0] == _claim(earlier, later) assert found.warnings[1].startswith(f"Timeline {earlier.id}: the last beat (3.0)") assert found.warnings[2].startswith(f"Timeline {later.id}: the last beat (3.0)") From 13d45efaba98c79f76815c7acf15404fddd86b92 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Thu, 15 Oct 2026 12:05:02 +0200 Subject: [PATCH 17/27] fix(core): type roles as holding lists too A role that is a list claims time_map, so the mapping's values are str | list[str] | None, as tla.Metadata's are. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/timemap/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/__init__.py b/packages/tilia-core/src/tilia_core/timemap/__init__.py index 0b07999da..e2c0ea2b8 100644 --- a/packages/tilia-core/src/tilia_core/timemap/__init__.py +++ b/packages/tilia-core/src/tilia_core/timemap/__init__.py @@ -38,7 +38,7 @@ class TimeMapFound: def find_time_map( - document: tla.Document, roles: Mapping[str, str | None] | None = None + document: tla.Document, roles: Mapping[str, str | list[str] | None] | None = None ) -> TimeMapFound: """The document's time map: its beat timeline whose role is time_map, else its first beat timeline by ordinal, guessed. @@ -104,7 +104,7 @@ def _by_ordinal(timeline: tla.Timeline) -> tuple[int, float]: def _claims_the_role( - timeline: tla.Timeline, roles: Mapping[str, str | None] | None + timeline: tla.Timeline, roles: Mapping[str, str | list[str] | None] | None ) -> bool: role = timeline.metadata.get("role") if roles is None else roles.get(timeline.id) if isinstance(role, (list, tuple)): @@ -113,7 +113,7 @@ def _claims_the_role( def time_map_of( - document: tla.Document, roles: Mapping[str, str | None] | None = None + document: tla.Document, roles: Mapping[str, str | list[str] | None] | None = None ) -> TimeMap | None: """The document's time map, chosen as `find_time_map` chooses it; None when it has none.""" return find_time_map(document, roles).time_map From ebba2d59131d401d14c1e933eb68501b20840785 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Thu, 15 Oct 2026 12:05:08 +0200 Subject: [PATCH 18/27] test(core): cite project-management issues in desktop's form A bare #7 links to desktop's own issue. The decisions are cited by the issue that records them. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../tilia-core/tests/timemap/test_timemap_choose.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 98cfc3777..a6748872a 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -135,7 +135,7 @@ def test_only_a_beat_timeline_can_have_the_role(): [(["time_map"], True), (["analysis", "time_map"], True), (["analysis"], False)], ) def test_a_role_list_that_holds_time_map_claims_it(role, claims): - # Anne, 2026-10-14: metadata values may be lists. + # Metadata values may be lists (TimeLineAnnotator/project-management#13 (the measure table)). first, listed = _beats(1), _beats(2) listed.metadata["role"] = role @@ -147,7 +147,8 @@ def test_a_role_list_that_holds_time_map_claims_it(role, claims): @pytest.mark.parametrize("ordinal", [None, "1", 1.5, float("nan"), True, [1]]) def test_an_ordinal_that_isnt_a_whole_number_sorts_after_the_others(ordinal): - # Anne, 2026-10-14: nothing raises while the reader lets one through (#7). + # Nothing raises while the reader lets one through + # (TimeLineAnnotator/project-management#7 (format in the core)). odd, other = _beats(1), _beats(3) odd.ordinal = ordinal @@ -270,8 +271,8 @@ def test_the_warnings_come_in_the_order_they_were_found(): def test_every_beat_timeline_past_the_media_is_warned_about(caplog): - # Anne, 2026-10-14: the tables a score or an import names too, once for - # each call of find_time_map. A table without rows has no closing barline. + # The tables a score or an import names too, once for each call of + # find_time_map (TimeLineAnnotator/project-management#13 (the measure table)). A table without rows has no closing barline. default, named = _beats(1, role="time_map"), _beats(2, last=7) timeless = _beats(3, last=7) timeless.components[timeless.measures.rows[0].beats[1]].attrs["time"] = None @@ -406,7 +407,7 @@ def _out_of_order(**kwargs) -> tla.Timeline: @pytest.mark.parametrize("make", [_same_time, _out_of_order]) def test_measure_rows_give_none_while_beats_are_out_of_order_or_share_a_time(make): - # Anne, 2026-10-14: no measure that ends before it starts. + # No measure that ends before it starts (TimeLineAnnotator/project-management#13 (the measure table)). timeline = make() assert measure_rows(document(timeline), timeline.id) == [] From 5edaaf8d341f4e137271e11a5da4e9c0b4aa837b Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:59:05 +0200 Subject: [PATCH 19/27] fix(core): treat a media length too large for a float as no length JSON reads a whole number of any size as an int, and the reader keeps a length of 400 digits, while it refuses 1e999. math.isfinite raises OverflowError on such an int, so find_time_map raised instead of reading the file without a length. A shared _is_number check now catches it, and media_warning doesn't read the length for a timeline without beats. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/table.py | 24 ++++++++++---- .../tests/timemap/test_timemap_choose.py | 31 +++++++++++++++++-- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/table.py b/packages/tilia-core/src/tilia_core/timemap/table.py index d8fd16b7b..50a7c4f9c 100644 --- a/packages/tilia-core/src/tilia_core/timemap/table.py +++ b/packages/tilia-core/src/tilia_core/timemap/table.py @@ -8,7 +8,7 @@ from collections.abc import Iterable, Mapping, Sequence from dataclasses import dataclass, field from fractions import Fraction -from typing import Any, NoReturn +from typing import Any, NoReturn, TypeGuard from tilia_core import tla from tilia_core.timemap._rows import RowIn, rows_of @@ -155,8 +155,10 @@ def media_warning(document: tla.Document, timeline: tla.Timeline) -> str | None: if table_problem(timeline) is not None: return None times = _times(timeline) + if not times: + return None media_length = _media_length(document) - if not times or media_length is None or not _ends_by(media_length, times[-1]): + if media_length is None or not _ends_by(media_length, times[-1]): return None warning = ( f"Timeline {timeline.id}: the last beat ({times[-1]}) is at or after " @@ -206,6 +208,17 @@ def _has_time(component: tla.Component | None) -> bool: ) +def _is_number(value: object) -> TypeGuard[int | float]: + """A finite number, as JSON stores one: not a bool, NaN, an infinity, or a + whole number too large for a float, which JSON reads as an int.""" + if isinstance(value, bool) or not isinstance(value, (int, float)): + return False + try: + return math.isfinite(value) + except OverflowError: + return False + + def build_rows_from( document: tla.Document, timeline: tla.Timeline, rows: Sequence[RowIn] ) -> list[MeasureRow]: @@ -296,12 +309,11 @@ def _media_length(document: tla.Document) -> float | None: It is in seconds, so it doesn't cap a quarters file. The format allows a number above zero, or none; the reader doesn't check, so anything else - (text, a bool, NaN, infinity, zero or less) counts as none. + (text, a bool, NaN, infinity, a whole number too large for a float, zero + or less) counts as none. """ length: object = document.media_length - if document.time_unit != "seconds" or isinstance(length, bool): - return None - if not isinstance(length, (int, float)) or not math.isfinite(length): + if document.time_unit != "seconds" or not _is_number(length): return None return length if length > 0 else None diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index a6748872a..daeb57945 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -306,11 +306,23 @@ def test_no_media_warning_when_the_media_lasts_past_the_last_beat( @pytest.mark.parametrize( - "media_length", ["20", True, float("nan"), float("inf"), 0, 0.0, -5.0, [19.5]] + "media_length", + [ + "20", + True, + float("nan"), + float("inf"), + pytest.param(10**400, id="10**400"), + 0, + 0.0, + -5.0, + [19.5], + ], ) def test_a_media_length_the_format_doesnt_allow_is_no_length(media_length, caplog): # The schema allows a number above zero, or null; the reader doesn't check. - # No cap on the closing barline, no warning, and nothing raises. + # No cap on the closing barline, no warning, and nothing raises. 10**400 + # is a whole number too large for a float. timeline = pickup_piece() doc = document(timeline, media_length=media_length) @@ -323,6 +335,21 @@ def test_a_media_length_the_format_doesnt_allow_is_no_length(media_length, caplo assert not caplog.records +def test_a_media_length_too_large_for_a_float_read_from_a_file_is_no_length(caplog): + # JSON reads a whole number of any size as an int, and the reader keeps + # it, while it refuses 1e999, which JSON reads as a float. + data = tla.canonical_bytes(document(pickup_piece(), media_length=20.4)) + assert data.count(b'"length": 20.4') == 1 + doc = tla.loads(data.replace(b'"length": 20.4', b'"length": 1' + b"0" * 400)) + assert doc.media_length == 10**400 + + with caplog.at_level(logging.WARNING): + found = find_time_map(doc) + + assert found.warnings == () + assert not caplog.records + + def test_one_beat_past_the_media_is_warned_about_without_a_gap(): # Its only measure ends on the beat: there is no gap after it. one = beat_timeline([{"time": 5.0, "measure": {}}]) From a0fe65727bc5c7252514a82b1cdd61cfce5582f9 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:59:16 +0200 Subject: [PATCH 20/27] fix(core): treat a beat time too large for a float as no time _has_time called math.isfinite, which raises OverflowError on a whole number too large for a float. Since find_time_map checks every beat timeline's table for the media warning, one such beat in any of them stopped the whole document. It now shares _is_number with the media length, so the beat has no time: its timeline alone has no rows and no time map, with "a beat without a time (measure N)". Part of TimeLineAnnotator/project-management#216 See TimeLineAnnotator/project-management#319 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/table.py | 10 +++------ .../tests/timemap/test_timemap_choose.py | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/table.py b/packages/tilia-core/src/tilia_core/timemap/table.py index 50a7c4f9c..b76ab211c 100644 --- a/packages/tilia-core/src/tilia_core/timemap/table.py +++ b/packages/tilia-core/src/tilia_core/timemap/table.py @@ -172,7 +172,8 @@ def media_warning(document: tla.Document, timeline: tla.Timeline) -> str | None: def missing_time(timeline: tla.Timeline) -> str | None: """The first beat without a usable time, as a reason naming its measure; else None. - A time is usable when it is a finite number: a missing, NaN or text time isn't. + A time is usable when it is a finite number: a missing, NaN or text time + isn't, nor a whole number too large for a float. """ if timeline.measures is None: return None @@ -200,12 +201,7 @@ def broken_row(timeline: tla.Timeline) -> str | None: def _has_time(component: tla.Component | None) -> bool: - time = component.attrs.get("time") if component is not None else None - return ( - isinstance(time, (int, float)) - and not isinstance(time, bool) - and math.isfinite(time) - ) + return component is not None and _is_number(component.attrs.get("time")) def _is_number(value: object) -> TypeGuard[int | float]: diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index daeb57945..8bc467f12 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -425,6 +425,28 @@ def test_measure_rows_give_none_while_a_beat_has_no_usable_time(): assert measure_rows(document(timeline), timeline.id) == [] +def test_a_beat_time_too_large_for_a_float_is_no_time(caplog): + # TimeLineAnnotator/project-management#319 (a beat time too large for a + # float): JSON reads a whole number of any size as an int, which a float + # can't hold. In a timeline that isn't the time map, it doesn't stop the + # document's: that timeline alone has no rows and no time map. + chosen, other = _beats(1, role="time_map"), _beats(2) + last = other.measures.rows[-1].beats[-1] + other.components[last].attrs["time"] = 10**400 + doc = document(chosen, other, media_length=2.0) + + with caplog.at_level(logging.WARNING): + found = find_time_map(doc) + + assert found.timeline_id == chosen.id and found.time_map is not None + (warning,) = found.warnings + assert warning.startswith(f"Timeline {chosen.id}: the last beat (3.0)") + assert len(caplog.records) == 1 + assert measure_rows(doc, other.id) == [] + assert time_map_for(doc, other.id) is None + assert find_time_map(document(other)).reason == "a beat without a time (measure 2)" + + def _out_of_order(**kwargs) -> tla.Timeline: return beat_timeline( [{"time": 0.0, "measure": {}}, {"time": 2.0, "measure": {}}, {"time": 1.0}], From 6ccc06927c41de56712b34fbd9be456bda288685 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:20:03 +0200 Subject: [PATCH 21/27] test(core): check the claimants' warning against its written text The expected warning was built by a copy of the code's own formatting, so a wrong join would pass. The tests now give the timelines fixed ids and spell the text out: "timelines A, B and C claim the role time_map; A, the first by ordinal, is used". Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../tests/timemap/test_timemap_choose.py | 47 ++++++++++++------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 8bc467f12..100237345 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -16,23 +16,16 @@ from tilia_core.timemap.timemap import FOLDED -def _claim(*claiming: tla.Timeline) -> str: - """The warning when several timelines claim the role, in ordinal order.""" - ids = [timeline.id for timeline in claiming] - named = f"{', '.join(ids[:-1])} and {ids[-1]}" - return ( - f"timelines {named} claim the role time_map; " - f"{ids[0]}, the first by ordinal, is used" - ) - - -def _beats(ordinal: int, role: str | None = None, *, last: int = 3) -> tla.Timeline: +def _beats( + ordinal: int, role: str | None = None, *, last: int = 3, id: str | None = None +) -> tla.Timeline: """2/4, a beat every second from 0 to `last`.""" return beat_timeline( [ {"time": float(t), "measure": {} if t % 2 == 0 else None} for t in range(last + 1) ], + id=id, ordinal=ordinal, role=role, ) @@ -75,13 +68,29 @@ def test_without_the_role_the_first_beat_timeline_by_ordinal_is_guessed(): assert found.reason is None and found.warnings == () -@pytest.mark.parametrize("claimants", [2, 3]) -def test_when_several_claim_the_role_the_first_by_ordinal_wins(claimants, caplog): +@pytest.mark.parametrize( + "claimants, warning", + [ + ( + 2, + "timelines B and C claim the role time_map; " + "B, the first by ordinal, is used", + ), + ( + 3, + "timelines A, B and C claim the role time_map; " + "A, the first by ordinal, is used", + ), + ], +) +def test_when_several_claim_the_role_the_first_by_ordinal_wins( + claimants, warning, caplog +): # Spec, US4 scenario 3: one warning, however many claim it, naming them in # ordinal order; returned and logged (TimeLineAnnotator/project-management#13 # (the measure table)). unclaimed = _beats(1) - claiming = [_beats(10 - j, role="time_map") for j in range(claimants)] + claiming = [_beats(10 - j, role="time_map", id="CBA"[j]) for j in range(claimants)] with caplog.at_level(logging.WARNING): found = find_time_map(document(unclaimed, *claiming)) @@ -89,7 +98,7 @@ def test_when_several_claim_the_role_the_first_by_ordinal_wins(claimants, caplog assert found.timeline_id == claiming[-1].id assert found.time_map.timeline_id == claiming[-1].id assert not found.guessed - assert found.warnings == (_claim(*reversed(claiming)),) + assert found.warnings == (warning,) (record,) = caplog.records assert record.name == "tilia_core.timemap" assert record.getMessage() == f"{found.warnings[0]}." @@ -260,12 +269,16 @@ def test_the_last_beat_at_or_after_the_end_of_the_media_is_returned_and_logged( def test_the_warnings_come_in_the_order_they_were_found(): # The claim first, then each timeline past the media, by ordinal. - later, earlier = _beats(2, role="time_map"), _beats(1, role="time_map") + later = _beats(2, role="time_map", id="late") + earlier = _beats(1, role="time_map", id="early") found = find_time_map(document(later, earlier, media_length=2.0)) assert len(found.warnings) == 3 - assert found.warnings[0] == _claim(earlier, later) + assert found.warnings[0] == ( + "timelines early and late claim the role time_map; " + "early, the first by ordinal, is used" + ) assert found.warnings[1].startswith(f"Timeline {earlier.id}: the last beat (3.0)") assert found.warnings[2].startswith(f"Timeline {later.id}: the last beat (3.0)") From 7f7844a35be66437a649b9f35bd3b864384cbb3e Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:20:13 +0200 Subject: [PATCH 22/27] test(core): a whole-number media length caps and warns JSON reads "length": 7 as an int, which the format allows, and no test used one: a length check narrowed to floats would have dropped the cap and the warning for such a file unnoticed. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../tests/timemap/test_timemap_choose.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 100237345..d9938f173 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -363,6 +363,24 @@ def test_a_media_length_too_large_for_a_float_read_from_a_file_is_no_length(capl assert not caplog.records +def test_a_whole_number_media_length_caps_and_warns(): + # JSON reads "length": 7 as an int, which the format allows. One measure of + # four beats, every 2 s: its closing barline is at 8. + timeline = beat_timeline( + [{"time": t, "measure": {} if t == 0.0 else None} for t in (0.0, 2.0, 4.0, 6.0)] + ) + + capped = find_time_map(document(timeline, media_length=7)) + past = find_time_map(document(timeline, media_length=6)) + + assert capped.time_map.end == 7 and capped.warnings == () + assert past.time_map.end == 8.0 + assert past.warnings == ( + f"Timeline {timeline.id}: the last beat (6.0) is at or after the end of the " + "media (6); the last measure ends one gap after it", + ) + + def test_one_beat_past_the_media_is_warned_about_without_a_gap(): # Its only measure ends on the beat: there is no gap after it. one = beat_timeline([{"time": 5.0, "measure": {}}]) From b65194842e0ea50030c7b59b1e69c731565a1031 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:20:21 +0200 Subject: [PATCH 23/27] refactor(core): one pass over a table in media_warning media_warning ran table_problem, which reads the table's times, then read them again. _checked gives the problem and the times together. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/table.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/table.py b/packages/tilia-core/src/tilia_core/timemap/table.py index b76ab211c..8e5562e3c 100644 --- a/packages/tilia-core/src/tilia_core/timemap/table.py +++ b/packages/tilia-core/src/tilia_core/timemap/table.py @@ -124,7 +124,18 @@ def table_problem(timeline: tla.Timeline) -> str | None: start at its downbeat (`broken_row`), or two beats at the same time or out of time order (`disorder`). """ - return missing_time(timeline) or broken_row(timeline) or disorder(_times(timeline)) + return _checked(timeline)[0] + + +def _checked(timeline: tla.Timeline) -> tuple[str | None, list[float]]: + # The table's problem, and the times of its beats in its order: none when it + # has a problem, so the times given are always usable. + reason = missing_time(timeline) or broken_row(timeline) + if reason is not None: + return reason, [] + times = _times(timeline) + reason = disorder(times) + return reason, [] if reason is not None else times def _times(timeline: tla.Timeline) -> list[float]: @@ -152,10 +163,8 @@ def media_warning(document: tla.Document, timeline: tla.Timeline) -> str | None: With more than one beat, the last measure ends one gap after the last beat, and the warning says so; with one, it ends on the beat. """ - if table_problem(timeline) is not None: - return None - times = _times(timeline) - if not times: + reason, times = _checked(timeline) + if reason is not None or not times: return None media_length = _media_length(document) if media_length is None or not _ends_by(media_length, times[-1]): From 523ad1d52f6cd453beab1094b67a10e164339185 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:20:29 +0200 Subject: [PATCH 24/27] refactor(core): one rule for the gap after the last beat The closing barline and the media warning each decided whether the last measure ends one gap after its last beat. _last_gap gives the last beat and the gap to both, so the warning can't describe a gap the barline doesn't have. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../tilia-core/src/tilia_core/timemap/table.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/table.py b/packages/tilia-core/src/tilia_core/timemap/table.py index 8e5562e3c..362d54bc5 100644 --- a/packages/tilia-core/src/tilia_core/timemap/table.py +++ b/packages/tilia-core/src/tilia_core/timemap/table.py @@ -166,14 +166,15 @@ def media_warning(document: tla.Document, timeline: tla.Timeline) -> str | None: reason, times = _checked(timeline) if reason is not None or not times: return None + last, gap = _last_gap(times) media_length = _media_length(document) - if media_length is None or not _ends_by(media_length, times[-1]): + if media_length is None or not _ends_by(media_length, last): return None warning = ( - f"Timeline {timeline.id}: the last beat ({times[-1]}) is at or after " + f"Timeline {timeline.id}: the last beat ({last}) is at or after " f"the end of the media ({media_length})" ) - if len(times) > 1: + if gap > 0: warning += "; the last measure ends one gap after it" return warning @@ -301,14 +302,20 @@ def is_folded(rows: Sequence[RowIn]) -> bool: def _closing_barline(document: tla.Document, rows: Sequence[RowIn]) -> float: # The last beat lasts as long as the gap before it, or until the end of the # media if that comes sooner. No beats are added to the last measure. - *before, last = [t for row in rows[-2:] for t in row.beat_times][-2:] - end = last + (last - before[0] if before else 0.0) + last, gap = _last_gap([t for row in rows[-2:] for t in row.beat_times]) + end = last + gap media_length = _media_length(document) if media_length is None or _ends_by(media_length, last): return end # past the media, the gap alone; find_time_map warns return min(end, media_length) +def _last_gap(times: Sequence[float]) -> tuple[float, float]: + # The last beat, and the gap before it, which the last measure lasts past + # it: none for a single beat. The closing barline and its warning share it. + return times[-1], (times[-1] - times[-2] if len(times) > 1 else 0.0) + + def _media_length(document: tla.Document) -> float | None: """The media's length, when it caps the closing barline; else None. From b3e8d77c2398e624ce278f888fe9b36a204e24cf Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:20:36 +0200 Subject: [PATCH 25/27] docs(core): say what table_problem's None does and doesn't promise A timeline without a table, or with an empty one, has no rows and no problem either. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/src/tilia_core/timemap/table.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/table.py b/packages/tilia-core/src/tilia_core/timemap/table.py index 362d54bc5..8e5c91c83 100644 --- a/packages/tilia-core/src/tilia_core/timemap/table.py +++ b/packages/tilia-core/src/tilia_core/timemap/table.py @@ -118,11 +118,12 @@ def build_rows(document: tla.Document, timeline: tla.Timeline) -> list[MeasureRo def table_problem(timeline: tla.Timeline) -> str | None: - """Why the timeline's table can have no rows, as a reason; else None. + """Why the timeline's table can't give rows, as a reason; else None. A beat without a usable time (`missing_time`), a row whose beats don't start at its downbeat (`broken_row`), or two beats at the same time or out - of time order (`disorder`). + of time order (`disorder`). None doesn't promise rows: a timeline without + a table, or with an empty one, has none, and no problem either. """ return _checked(timeline)[0] From f71abfa40ada479aa80ff0055d112f601fd95197 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:20:43 +0200 Subject: [PATCH 26/27] test(core): take the timeline factories directly, not through globals() _out_of_order moves next to _same_time, so the media test can name both, and a rename shows up where they're used. Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- .../tests/timemap/test_timemap_choose.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index d9938f173..7347f8bd7 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -198,6 +198,13 @@ def _same_time(**kwargs) -> tla.Timeline: ) +def _out_of_order(**kwargs) -> tla.Timeline: + return beat_timeline( + [{"time": 0.0, "measure": {}}, {"time": 2.0, "measure": {}}, {"time": 1.0}], + **kwargs, + ) + + @pytest.mark.parametrize( "make, reason", [ @@ -394,10 +401,10 @@ def test_one_beat_past_the_media_is_warned_about_without_a_gap(): ) -@pytest.mark.parametrize("make", ["_same_time", "_out_of_order"]) +@pytest.mark.parametrize("make", [_same_time, _out_of_order]) def test_a_table_out_of_time_order_gets_no_media_warning(make, caplog): # It has no rows, so no closing barline. - timeline = globals()[make]() + timeline = make() doc = document(timeline, media_length=0.5) with caplog.at_level(logging.WARNING): @@ -478,13 +485,6 @@ def test_a_beat_time_too_large_for_a_float_is_no_time(caplog): assert find_time_map(document(other)).reason == "a beat without a time (measure 2)" -def _out_of_order(**kwargs) -> tla.Timeline: - return beat_timeline( - [{"time": 0.0, "measure": {}}, {"time": 2.0, "measure": {}}, {"time": 1.0}], - **kwargs, - ) - - @pytest.mark.parametrize("make", [_same_time, _out_of_order]) def test_measure_rows_give_none_while_beats_are_out_of_order_or_share_a_time(make): # No measure that ends before it starts (TimeLineAnnotator/project-management#13 (the measure table)). From 27ae7f73afc7537df69995d9ceed917592203623 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:20:51 +0200 Subject: [PATCH 27/27] test(core): wrap three comments at 88 columns Part of TimeLineAnnotator/project-management#216 Co-Authored-By: Claude Opus 5.5 --- packages/tilia-core/tests/timemap/test_timemap_choose.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/tilia-core/tests/timemap/test_timemap_choose.py b/packages/tilia-core/tests/timemap/test_timemap_choose.py index 7347f8bd7..f9c426dba 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_choose.py +++ b/packages/tilia-core/tests/timemap/test_timemap_choose.py @@ -144,7 +144,8 @@ def test_only_a_beat_timeline_can_have_the_role(): [(["time_map"], True), (["analysis", "time_map"], True), (["analysis"], False)], ) def test_a_role_list_that_holds_time_map_claims_it(role, claims): - # Metadata values may be lists (TimeLineAnnotator/project-management#13 (the measure table)). + # Metadata values may be lists (TimeLineAnnotator/project-management#13 + # (the measure table)). first, listed = _beats(1), _beats(2) listed.metadata["role"] = role @@ -292,7 +293,8 @@ def test_the_warnings_come_in_the_order_they_were_found(): def test_every_beat_timeline_past_the_media_is_warned_about(caplog): # The tables a score or an import names too, once for each call of - # find_time_map (TimeLineAnnotator/project-management#13 (the measure table)). A table without rows has no closing barline. + # find_time_map (TimeLineAnnotator/project-management#13 (the measure + # table)). A table without rows has no closing barline. default, named = _beats(1, role="time_map"), _beats(2, last=7) timeless = _beats(3, last=7) timeless.components[timeless.measures.rows[0].beats[1]].attrs["time"] = None @@ -487,7 +489,8 @@ def test_a_beat_time_too_large_for_a_float_is_no_time(caplog): @pytest.mark.parametrize("make", [_same_time, _out_of_order]) def test_measure_rows_give_none_while_beats_are_out_of_order_or_share_a_time(make): - # No measure that ends before it starts (TimeLineAnnotator/project-management#13 (the measure table)). + # No measure that ends before it starts + # (TimeLineAnnotator/project-management#13 (the measure table)). timeline = make() assert measure_rows(document(timeline), timeline.id) == []