From f2fedc5ab2d2bc082907696600f4857cc865b328 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Fri, 9 Oct 2026 15:16:13 +0200 Subject: [PATCH 1/5] feat(core): move the beat unit rules into tilia_core.timemap.units Moved from tilia/timelines/beat/units.py (desktop#621), unchanged: the file is the same blob as at a17bbd83. TestParseUnits and TestFitUnits are ported case for case to test_timemap_units.py, with a test of validate_denominator. Part of TimeLineAnnotator/project-management#213 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/units.py | 121 ++++++++++++++++++ .../tests/timemap/test_timemap_units.py | 95 ++++++++++++++ 2 files changed, 216 insertions(+) create mode 100644 packages/tilia-core/src/tilia_core/timemap/units.py create mode 100644 packages/tilia-core/tests/timemap/test_timemap_units.py diff --git a/packages/tilia-core/src/tilia_core/timemap/units.py b/packages/tilia-core/src/tilia_core/timemap/units.py new file mode 100644 index 000000000..7295d85d6 --- /dev/null +++ b/packages/tilia-core/src/tilia_core/timemap/units.py @@ -0,0 +1,121 @@ +""" +Beat units: what one tapped beat is worth in notation. + +A beat unit is a denominator plus a units pattern, the number of +denominator units each tapped beat in a measure is worth. Units are written +as a "+"-separated list of whole numbers or fractions: "1", "3", "2+3", +"1/2", "1/3". The time signature of a measure is calculated from its beat +unit and number of tapped beats, and is never stored. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from fractions import Fraction + +DEFAULT_DENOMINATOR = 4 +DEFAULT_UNITS = "1" +# Far above any denominator used in notation. Enforced wherever a beat unit +# is set, so the dialog and inspector, which can't show more, never meet one. +MAX_DENOMINATOR = 128 + +_UNIT_CHARACTERS = set("0123456789/") + + +def validate_denominator(value: int) -> bool: + return ( + isinstance(value, int) + and not isinstance(value, bool) + and 1 <= value <= MAX_DENOMINATOR + ) + + +@dataclass(frozen=True) +class UnitsParseResult: + units: list[Fraction] + error: str = "" + + @property + def is_valid(self) -> bool: + return not self.error + + +def parse_units(text: str) -> UnitsParseResult: + """Parses units text such as "2+3" or "1/3" into fractions.""" + if not text.strip(): + return UnitsParseResult([], "Enter at least one unit.") + + units = [] + for item in text.split("+"): + item = item.strip() + if not item: + return UnitsParseResult([], "Missing unit around '+'.") + not_a_fraction = UnitsParseResult( + [], f"'{item}' is not a whole number or fraction such as 1/2." + ) + # Fraction() also accepts decimals, signs and exponents; units are + # written the way they are in notation. + if not set(item) <= _UNIT_CHARACTERS: + return not_a_fraction + try: + unit = Fraction(item) + except (ValueError, ZeroDivisionError): + return not_a_fraction + if unit <= 0: + return UnitsParseResult([], "Units must be greater than 0.") + units.append(unit) + + return UnitsParseResult(units) + + +def format_units(units: list[Fraction]) -> str: + """Writes units in canonical form, e.g. [2, 3] as "2+3".""" + return "+".join(str(unit) for unit in units) + + +def fit_units(units: list[Fraction], beat_count: int) -> list[Fraction]: + """ + The value of each of `beat_count` tapped beats in a measure. The pattern + is read from its start in every measure, pickups included: a shorter + measure takes its first entries and a longer one repeats it. A pickup + that holds the end of the pattern needs a beat unit of its own. + """ + return [units[i % len(units)] for i in range(beat_count)] + + +def is_fit_ambiguous(units: list[Fraction], beat_count: int) -> bool: + """ + Whether the measure doesn't hold a whole number of patterns: the pattern + is not uniform and the beat count is not a multiple of its length. One + tap under 2+3 is read as 2, but could be a pickup worth 3. + """ + return len(set(units)) > 1 and beat_count % len(units) != 0 + + +@dataclass(frozen=True) +class MeasureMeter: + """ + The meter a measure ends up with. `beat_unit_id` is the beat unit that + governs the measure, or None where the default applies. `is_assumed` is + True when its values weren't set by the user (see BeatUnit.assumed). + """ + + denominator: int + units: list[Fraction] + beat_values: list[Fraction] + beat_unit_id: int | None + starts_here: bool + is_ambiguous: bool + is_conflicting: bool + is_assumed: bool + + @property + def numerator(self) -> Fraction: + return sum(self.beat_values, Fraction(0)) + + @property + def time_signature(self) -> tuple[Fraction, int]: + return self.numerator, self.denominator + + def quarter_length(self, beat_value: Fraction) -> Fraction: + return beat_value * 4 / self.denominator diff --git a/packages/tilia-core/tests/timemap/test_timemap_units.py b/packages/tilia-core/tests/timemap/test_timemap_units.py new file mode 100644 index 000000000..4eba3a216 --- /dev/null +++ b/packages/tilia-core/tests/timemap/test_timemap_units.py @@ -0,0 +1,95 @@ +from fractions import Fraction as F + +import pytest + +from tilia_core.timemap.units import ( + MAX_DENOMINATOR, + fit_units, + format_units, + is_fit_ambiguous, + parse_units, + validate_denominator, +) + + +class TestParseUnits: + @pytest.mark.parametrize( + "text, units", + [ + ("1", [F(1)]), + ("3", [F(3)]), + ("2+3", [F(2), F(3)]), + (" 2 + 2 + 3 ", [F(2), F(2), F(3)]), + ("1/2", [F(1, 2)]), + ("1/3+2/3", [F(1, 3), F(2, 3)]), + ("2/4", [F(1, 2)]), + ], + ) + def test_valid(self, text, units): + result = parse_units(text) + assert result.is_valid + assert result.units == units + + @pytest.mark.parametrize( + "text", ["", " ", "2+", "+2", "2++3", "0", "1/0", "1.5", "-1", "a", "1e2"] + ) + def test_invalid(self, text): + result = parse_units(text) + assert not result.is_valid + assert result.error + assert result.units == [] + + def test_format(self): + assert format_units([F(2), F(3)]) == "2+3" + assert format_units([F(1, 3)]) == "1/3" + assert format_units(parse_units(" 2/4 + 1 ").units) == "1/2+1" + + +class TestFitUnits: + @pytest.mark.parametrize( + "units, beat_count, values", + [ + ([F(1)], 4, [F(1)] * 4), + ([F(2), F(3)], 2, [F(2), F(3)]), + ([F(2), F(3)], 4, [F(2), F(3), F(2), F(3)]), + ([F(2), F(3)], 3, [F(2), F(3), F(2)]), + ([F(3), F(2)], 1, [F(3)]), + ([F(2), F(2), F(3)], 2, [F(2), F(2)]), + ([F(1), F(2), F(3)], 2, [F(1), F(2)]), + ([F(1)], 0, []), + ], + ) + def test_fit(self, units, beat_count, values): + assert fit_units(units, beat_count) == values + + @pytest.mark.parametrize( + "units, beat_count, ambiguous", + [ + ([F(1)], 3, False), + ([F(3)], 1, False), + ([F(1, 2)], 3, False), + ([F(2), F(3)], 2, False), + ([F(2), F(3)], 4, False), + ([F(2), F(3)], 1, True), + ([F(2), F(3)], 3, True), + ([F(2), F(2)], 3, False), + ], + ) + def test_ambiguous(self, units, beat_count, ambiguous): + assert is_fit_ambiguous(units, beat_count) == ambiguous + + +@pytest.mark.parametrize( + "value, valid", + [ + (1, True), + (8, True), + (MAX_DENOMINATOR, True), + (0, False), + (MAX_DENOMINATOR + 1, False), + (True, False), + (4.0, False), + ], +) +def test_validate_denominator(value, valid): + assert validate_denominator(value) == valid From 9f19a1a53b214153d759c85e432b1bc0dbdd058b Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Fri, 9 Oct 2026 15:20:15 +0200 Subject: [PATCH 2/5] feat(core): compute each measure's meter with meters_of(rows) meters_of(rows) is the loop of BeatTimeline._compute_measure_meters (desktop#621), reading the units set on each row's beats: the first unit in a measure governs it and is carried forward, any other makes it conflicting, and before any unit a measure of N taps is N/4, assumed. beat_unit_id is the id of the beat that carries the governing unit, since a beat unit in a .tla file has no id of its own. A unit desktop#621 refuses to create (units parse_units rejects, a denominator out of range) is logged and ignored, so the unit before it carries on, as when TiLiA drops it from a file it loads. TestMeasureMeters is ported to test_timemap_meters.py, apart from the cases about editing in the app, with tests of the default, of assumed, of unreadable units and of the acceptance scenarios of the time signatures. Part of TimeLineAnnotator/project-management#213 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/meters.py | 101 +++++++++ .../tests/timemap/test_timemap_meters.py | 207 ++++++++++++++++++ 2 files changed, 308 insertions(+) create mode 100644 packages/tilia-core/src/tilia_core/timemap/meters.py create mode 100644 packages/tilia-core/tests/timemap/test_timemap_meters.py diff --git a/packages/tilia-core/src/tilia_core/timemap/meters.py b/packages/tilia-core/src/tilia_core/timemap/meters.py new file mode 100644 index 000000000..38614efac --- /dev/null +++ b/packages/tilia-core/src/tilia_core/timemap/meters.py @@ -0,0 +1,101 @@ +"""Each measure's meter, from the beat units set on its beats, with desktop#621's rules.""" + +from __future__ import annotations + +import logging +from collections.abc import Sequence +from dataclasses import dataclass +from fractions import Fraction + +from tilia_core import tla +from tilia_core.timemap._rows import RowIn +from tilia_core.timemap.units import ( + DEFAULT_DENOMINATOR, + DEFAULT_UNITS, + MeasureMeter, + fit_units, + is_fit_ambiguous, + parse_units, + validate_denominator, +) + +logger = logging.getLogger(__name__) + + +@dataclass(frozen=True) +class _BeatUnit: + """A beat unit as desktop#621's component holds it, with its beat's id as its id.""" + + id: str + denominator: int + units: str + assumed: bool + + @property + def unit_values(self) -> list[Fraction]: + return parse_units(self.units).units + + +def meters_of(rows: Sequence[RowIn]) -> list[MeasureMeter]: + """ + The meter of every measure, from the beat units in effect: the loop of + desktop#621's `BeatTimeline._compute_measure_meters`. The first unit set + in a measure governs it and is carried forward; any other in that measure + makes it conflicting. Until a unit is set, a measure of N taps is N/4, + assumed. A unit that can't be read is ignored, as if absent. + """ + default_units = parse_units(DEFAULT_UNITS).units + + meters = [] + governing: _BeatUnit | None = None + for row in rows: + beat_count = len(row.beat_ids) + in_measure = [ + beat_unit + for beat_id, unit in row.units + if (beat_unit := _readable(beat_id, unit)) is not None + ] + if in_measure: + governing = in_measure[0] + + if governing is None: + denominator, units = DEFAULT_DENOMINATOR, default_units + else: + denominator, units = governing.denominator, governing.unit_values + + meters.append( + MeasureMeter( + denominator=denominator, + units=units, + beat_values=fit_units(units, beat_count), + beat_unit_id=governing.id if governing else None, + starts_here=bool(in_measure), + is_ambiguous=is_fit_ambiguous(units, beat_count), + is_conflicting=len(in_measure) > 1, + is_assumed=governing is None or governing.assumed, + ) + ) + return meters + + +def _readable(beat_id: str, unit: tla.BeatUnit) -> _BeatUnit | None: + # desktop#621 refuses to create such a unit, and so drops it from a file + # it loads: the unit before carries on. + if ( + not isinstance(unit.units, str) + or not parse_units(unit.units).is_valid + or not validate_denominator(unit.denominator) + ): + logger.warning( + "Beat %s: the beat unit %r over %r can't be read; ignored.", + beat_id, + unit.units, + unit.denominator, + ) + return None + return _BeatUnit( + id=beat_id, + denominator=unit.denominator, + units=unit.units, + assumed=unit.assumed, + ) diff --git a/packages/tilia-core/tests/timemap/test_timemap_meters.py b/packages/tilia-core/tests/timemap/test_timemap_meters.py new file mode 100644 index 000000000..8703271c6 --- /dev/null +++ b/packages/tilia-core/tests/timemap/test_timemap_meters.py @@ -0,0 +1,207 @@ +from fractions import Fraction as F + +import pytest + +from docs import beat_timeline +from tilia_core.timemap._rows import rows_of +from tilia_core.timemap.meters import meters_of + +# What TiLiA puts on the first beat, and WP3's migration on an old file's. +DEFAULT = {"denominator": 4, "units": "1", "assumed": True} + + +def unit(denominator, units, assumed=False): + return {"denominator": denominator, "units": units, "assumed": assumed} + + +def tapped(count, pattern=(4,), units=None): + """ + `count` beats a second apart, in measures of `pattern` beats, repeated as + a beat pattern is, with `units` ({beat index: unit}) set on beats. The first + beat has the default unit, assumed, unless `units` says otherwise. + """ + units = {0: DEFAULT, **(units or {})} + starts, start, i = set(), 0, 0 + while start < count: + starts.add(start) + start += pattern[i % len(pattern)] + i += 1 + return beat_timeline( + [ + { + "time": float(index), + "measure": {} if index in starts else None, + "beat_unit": units.get(index), + } + for index in range(count) + ] + ) + + +def beat_ids(timeline): + return list(timeline.components) + + +def meters(timeline): + return meters_of(rows_of(timeline)) + + +def time_signatures(timeline): + return [(m.numerator, m.denominator) for m in meters(timeline)] + + +class TestMeasureMeters: + def test_first_beat_gets_the_default_beat_unit(self): + timeline = tapped(7) + first = beat_ids(timeline)[0] + + assert time_signatures(timeline) == [(4, 4), (3, 4)] + assert [m.beat_unit_id for m in meters(timeline)] == [first] * 2 + assert [m.starts_here for m in meters(timeline)] == [True, False] + + def test_empty_timeline(self): + assert meters_of([]) == [] + + def test_beat_unit_applies_until_next_change(self): + timeline = tapped(8, (2,), {2: unit(8, "3")}) + first, beat_unit = beat_ids(timeline)[0], beat_ids(timeline)[2] + + assert time_signatures(timeline) == [(2, 4), (6, 8), (6, 8), (6, 8)] + result = meters(timeline) + assert [m.starts_here for m in result] == [True, True, False, False] + assert [m.beat_unit_id for m in result] == [first] + [beat_unit] * 3 + + def test_beat_unit_on_a_later_beat_applies_from_its_measure(self): + timeline = tapped(8, units={6: unit(8, "1")}) + + assert time_signatures(timeline) == [(4, 4), (4, 8)] + + def test_numerator_follows_bar_length(self): + timeline = tapped(15, (5, 7, 3), {0: unit(8, "1")}) + + assert time_signatures(timeline) == [(5, 8), (7, 8), (3, 8)] + + def test_fractional_units(self): + timeline = tapped(7, (4, 3), {0: unit(2, "1/2")}) + + assert time_signatures(timeline) == [(2, 2), (F(3, 2), 2)] + + def test_ambiguous_fit(self): + timeline = tapped(5, (2, 3), {0: unit(4, "2+3")}) + + result = meters(timeline) + assert [m.beat_values for m in result] == [[2, 3], [2, 3, 2]] + assert [m.is_ambiguous for m in result] == [False, True] + + def test_short_measures_take_the_start_of_the_pattern(self): + timeline = tapped(6, (2,), {0: unit(4, "1+2+3")}) + + result = meters(timeline) + assert [m.beat_values for m in result] == [[1, 2]] * 3 + assert all(m.is_ambiguous for m in result) + + def test_pickup_also_takes_the_start_of_the_pattern(self): + timeline = tapped(3, (1, 2), {0: unit(4, "3+2")}) + + result = meters(timeline) + assert result[0].beat_values == [3] + assert result[1].beat_values == [3, 2] + + def test_two_beat_units_in_a_measure_first_wins(self): + timeline = tapped(8, units={4: unit(8, "1"), 6: unit(2, "1")}) + + meter = meters(timeline)[1] + assert meter.is_conflicting + assert meter.beat_unit_id == beat_ids(timeline)[4] + assert meter.denominator == 8 + + +class TestDefaultAndAssumed: + def test_without_any_unit_a_measure_of_n_taps_is_n_over_4(self): + timeline = tapped(7, units={0: None}) + + result = meters(timeline) + assert time_signatures(timeline) == [(4, 4), (3, 4)] + assert [m.units for m in result] == [[1], [1]] + assert [m.beat_unit_id for m in result] == [None, None] + assert [m.starts_here for m in result] == [False, False] + assert all(m.is_assumed for m in result) + + def test_assumed_follows_the_governing_unit(self): + timeline = tapped(12, units={4: unit(8, "1"), 8: unit(8, "2", assumed=True)}) + + assert [m.is_assumed for m in meters(timeline)] == [True, False, True] + + def test_the_first_unit_of_a_conflicting_measure_is_carried_forward(self): + timeline = tapped(12, units={4: unit(8, "1"), 6: unit(2, "1")}) + + result = meters(timeline) + assert [m.is_conflicting for m in result] == [False, True, False] + assert result[2].beat_unit_id == beat_ids(timeline)[4] + assert result[2].time_signature == (4, 8) + + +class TestUnreadableUnits: + @pytest.mark.parametrize( + "bad", + [ + unit(8, "a"), + unit(8, "0"), + unit(8, ""), + unit(8, "2+"), + unit(8, 3), + unit(0, "1"), + unit(129, "1"), + ], + ) + def test_is_ignored_and_the_unit_before_carries_on(self, bad, caplog): + timeline = tapped(8, (2,), {2: unit(8, "3"), 4: bad}) + before = beat_ids(timeline)[2] + + result = meters(timeline) + assert time_signatures(timeline) == [(2, 4), (6, 8), (6, 8), (6, 8)] + assert [m.starts_here for m in result] == [True, True, False, False] + assert [m.beat_unit_id for m in result][1:] == [before] * 3 + assert "beat unit" in caplog.text + + def test_does_not_make_its_measure_conflicting(self): + timeline = tapped(4, units={0: unit(8, "3"), 2: unit(8, "x")}) + + (meter,) = meters(timeline) + assert not meter.is_conflicting + assert meter.time_signature == (12, 8) + + +THIRDS = [F(1, 3)] * 4 + + +class TestUserStory2: + @pytest.mark.parametrize( + "taps, beat_unit, time_signature, beat_values, quarters, assumed, ambiguous", + [ + # A measure of two taps with only the default unit: 2/4, assumed. + (2, DEFAULT, (2, 4), [1, 1], [1, 1], True, False), + # 6/8 tapped in two. + (2, unit(8, "3"), (6, 8), [3, 3], [F(3, 2), F(3, 2)], False, False), + # 5/8 tapped as 2+3. + (2, unit(8, "2+3"), (5, 8), [2, 3], [1, F(3, 2)], False, False), + # Three taps under 2+3: 7/8, ambiguous. + (3, unit(8, "2+3"), (7, 8), [2, 3, 2], [1, F(3, 2), 1], False, True), + # A pickup of one tap under {8, "3"}: 3/8. + (1, unit(8, "3"), (3, 8), [3], [F(3, 2)], False, False), + # Units 1/3 over 4 on four taps: a numerator that isn't whole. + (4, unit(4, "1/3"), (F(4, 3), 4), THIRDS, THIRDS, False, False), + ], + ) + def test_scenario( + self, taps, beat_unit, time_signature, beat_values, quarters, assumed, ambiguous + ): + (meter,) = meters(tapped(taps, (taps,), {0: beat_unit})) + + assert meter.time_signature == time_signature + assert meter.beat_values == beat_values + assert [meter.quarter_length(value) for value in meter.beat_values] == quarters + assert meter.is_assumed == assumed + assert meter.is_ambiguous == ambiguous + assert all(isinstance(value, F) for value in meter.beat_values) + assert isinstance(meter.numerator, F) From 1b8fd22dd19b3609e001da6b2173be0f4e48e4b2 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Fri, 9 Oct 2026 16:52:27 +0200 Subject: [PATCH 3/5] refactor(core): parse a beat unit once in meters_of, and test its warning meters_of parsed the governing unit's text again for every measure it governs; it is now parsed once, when it is checked, and each meter gets its own list. The tests copy each unit's dict into the timeline, check that the warning names the ignored beat, and cover an unreadable unit before a readable one in the same measure. Part of TimeLineAnnotator/project-management#213 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/meters.py | 28 +++++++++++-------- .../tests/timemap/test_timemap_meters.py | 21 +++++++++++--- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/meters.py b/packages/tilia-core/src/tilia_core/timemap/meters.py index 38614efac..9347fd968 100644 --- a/packages/tilia-core/src/tilia_core/timemap/meters.py +++ b/packages/tilia-core/src/tilia_core/timemap/meters.py @@ -1,4 +1,8 @@ -"""Each measure's meter, from the beat units set on its beats, with desktop#621's rules.""" +"""Each measure's meter, from the beat units set on its beats. + +The rules are those of desktop#621 (beat units and time signatures): its +`BeatTimeline._compute_measure_meters`, moved, with `units.py` beside it. +""" from __future__ import annotations @@ -24,17 +28,16 @@ @dataclass(frozen=True) class _BeatUnit: - """A beat unit as desktop#621's component holds it, with its beat's id as its id.""" + """A beat unit as desktop#621's component holds it, with its beat's id as its id. + + Its units are parsed once, when they are checked. + """ id: str denominator: int - units: str + unit_values: tuple[Fraction, ...] assumed: bool - @property - def unit_values(self) -> list[Fraction]: - return parse_units(self.units).units - def meters_of(rows: Sequence[RowIn]) -> list[MeasureMeter]: """ @@ -59,9 +62,9 @@ def meters_of(rows: Sequence[RowIn]) -> list[MeasureMeter]: governing = in_measure[0] if governing is None: - denominator, units = DEFAULT_DENOMINATOR, default_units + denominator, units = DEFAULT_DENOMINATOR, list(default_units) else: - denominator, units = governing.denominator, governing.unit_values + denominator, units = governing.denominator, list(governing.unit_values) meters.append( MeasureMeter( @@ -81,9 +84,10 @@ def meters_of(rows: Sequence[RowIn]) -> list[MeasureMeter]: def _readable(beat_id: str, unit: tla.BeatUnit) -> _BeatUnit | None: # desktop#621 refuses to create such a unit, and so drops it from a file # it loads: the unit before carries on. + parsed = parse_units(unit.units) if isinstance(unit.units, str) else None if ( - not isinstance(unit.units, str) - or not parse_units(unit.units).is_valid + parsed is None + or not parsed.is_valid or not validate_denominator(unit.denominator) ): logger.warning( @@ -96,6 +100,6 @@ def _readable(beat_id: str, unit: tla.BeatUnit) -> _BeatUnit | None: return _BeatUnit( id=beat_id, denominator=unit.denominator, - units=unit.units, + unit_values=tuple(parsed.units), assumed=unit.assumed, ) diff --git a/packages/tilia-core/tests/timemap/test_timemap_meters.py b/packages/tilia-core/tests/timemap/test_timemap_meters.py index 8703271c6..b96be3846 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_meters.py +++ b/packages/tilia-core/tests/timemap/test_timemap_meters.py @@ -31,7 +31,8 @@ def tapped(count, pattern=(4,), units=None): { "time": float(index), "measure": {} if index in starts else None, - "beat_unit": units.get(index), + # A copy, so that no two tests share a unit's dict. + "beat_unit": dict(units[index]) if units.get(index) else None, } for index in range(count) ] @@ -156,13 +157,25 @@ class TestUnreadableUnits: ) def test_is_ignored_and_the_unit_before_carries_on(self, bad, caplog): timeline = tapped(8, (2,), {2: unit(8, "3"), 4: bad}) - before = beat_ids(timeline)[2] + before, ignored = beat_ids(timeline)[2], beat_ids(timeline)[4] result = meters(timeline) - assert time_signatures(timeline) == [(2, 4), (6, 8), (6, 8), (6, 8)] + assert [m.time_signature for m in result] == [(2, 4), (6, 8), (6, 8), (6, 8)] assert [m.starts_here for m in result] == [True, True, False, False] assert [m.beat_unit_id for m in result][1:] == [before] * 3 - assert "beat unit" in caplog.text + (warning,) = [ + r for r in caplog.records if r.name == "tilia_core.timemap.meters" + ] + assert ignored in warning.getMessage() + + def test_before_a_readable_one_does_not_govern_its_measure(self): + timeline = tapped(8, units={4: unit(8, "x"), 6: unit(2, "1")}) + + meter = meters(timeline)[1] + assert meter.beat_unit_id == beat_ids(timeline)[6] + assert meter.denominator == 2 + assert meter.starts_here + assert not meter.is_conflicting def test_does_not_make_its_measure_conflicting(self): timeline = tapped(4, units={0: unit(8, "3"), 2: unit(8, "x")}) From 20b9931a20045baa0eacce804cb341eafd143e6f Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Fri, 9 Oct 2026 17:56:24 +0200 Subject: [PATCH 4/5] refactor(core): leave out unreadable beat units once, in rows_of rows_of now leaves out a beat unit desktop#621 (beat units and time signatures) would refuse to create: units parse_units rejects, or a denominator outside 1-128. Every reader of RowIn.units sees only readable units, and the warning names the timeline and the beat once per read, with the unit shortened. meters_of no longer checks or logs. Part of TimeLineAnnotator/project-management#213 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/_rows.py | 19 +++++++++- .../src/tilia_core/timemap/meters.py | 34 +++-------------- .../tests/timemap/test_timemap_meters.py | 6 +-- .../tests/timemap/test_timemap_rows.py | 38 +++++++++++++++++++ 4 files changed, 64 insertions(+), 33 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/_rows.py b/packages/tilia-core/src/tilia_core/timemap/_rows.py index a2fb8d830..a65370029 100644 --- a/packages/tilia-core/src/tilia_core/timemap/_rows.py +++ b/packages/tilia-core/src/tilia_core/timemap/_rows.py @@ -7,6 +7,7 @@ from typing import Any from tilia_core import tla +from tilia_core.timemap.units import parse_units, validate_denominator logger = logging.getLogger(__name__) @@ -93,19 +94,33 @@ def _dict(component: tla.Component | None, key: str) -> dict[str, Any]: def _beat_unit(timeline: tla.Timeline, component: tla.Component) -> tla.BeatUnit | None: + """The unit set on a beat; None, with a warning, when it can't be read.""" value = component.attrs.get("beat_unit") if value is None: return None try: - return tla.BeatUnit( + unit: tla.BeatUnit | None = tla.BeatUnit( denominator=value["denominator"], units=value["units"], assumed=bool(value.get("assumed", False)), ) except (KeyError, TypeError): + unit = None + if unit is None or not _readable(unit): logger.warning( - "Timeline %s: beat %s has a beat unit that can't be read; ignored.", + "Timeline %s: beat %s has a beat unit that can't be read (%.60r); ignored.", timeline.id, component.id, + value, ) return None + return unit + + +def _readable(unit: tla.BeatUnit) -> bool: + # What desktop#621 refuses to create, and so drops from a file it loads. + return ( + isinstance(unit.units, str) + and parse_units(unit.units).is_valid + and validate_denominator(unit.denominator) + ) diff --git a/packages/tilia-core/src/tilia_core/timemap/meters.py b/packages/tilia-core/src/tilia_core/timemap/meters.py index 9347fd968..56433d0a1 100644 --- a/packages/tilia-core/src/tilia_core/timemap/meters.py +++ b/packages/tilia-core/src/tilia_core/timemap/meters.py @@ -6,7 +6,6 @@ from __future__ import annotations -import logging from collections.abc import Sequence from dataclasses import dataclass from fractions import Fraction @@ -20,17 +19,14 @@ fit_units, is_fit_ambiguous, parse_units, - validate_denominator, ) -logger = logging.getLogger(__name__) - @dataclass(frozen=True) class _BeatUnit: """A beat unit as desktop#621's component holds it, with its beat's id as its id. - Its units are parsed once, when they are checked. + Its units are parsed once for each call of `meters_of`. """ id: str @@ -45,7 +41,8 @@ def meters_of(rows: Sequence[RowIn]) -> list[MeasureMeter]: desktop#621's `BeatTimeline._compute_measure_meters`. The first unit set in a measure governs it and is carried forward; any other in that measure makes it conflicting. Until a unit is set, a measure of N taps is N/4, - assumed. A unit that can't be read is ignored, as if absent. + assumed. `rows_of` has already left out the units that can't be read, so + the unit before such a unit carries on. """ default_units = parse_units(DEFAULT_UNITS).units @@ -53,11 +50,7 @@ def meters_of(rows: Sequence[RowIn]) -> list[MeasureMeter]: governing: _BeatUnit | None = None for row in rows: beat_count = len(row.beat_ids) - in_measure = [ - beat_unit - for beat_id, unit in row.units - if (beat_unit := _readable(beat_id, unit)) is not None - ] + in_measure = [_parsed(beat_id, unit) for beat_id, unit in row.units] if in_measure: governing = in_measure[0] @@ -81,25 +74,10 @@ def meters_of(rows: Sequence[RowIn]) -> list[MeasureMeter]: return meters -def _readable(beat_id: str, unit: tla.BeatUnit) -> _BeatUnit | None: - # desktop#621 refuses to create such a unit, and so drops it from a file - # it loads: the unit before carries on. - parsed = parse_units(unit.units) if isinstance(unit.units, str) else None - if ( - parsed is None - or not parsed.is_valid - or not validate_denominator(unit.denominator) - ): - logger.warning( - "Beat %s: the beat unit %r over %r can't be read; ignored.", - beat_id, - unit.units, - unit.denominator, - ) - return None +def _parsed(beat_id: str, unit: tla.BeatUnit) -> _BeatUnit: return _BeatUnit( id=beat_id, denominator=unit.denominator, - unit_values=tuple(parsed.units), + unit_values=tuple(parse_units(unit.units).units), assumed=unit.assumed, ) diff --git a/packages/tilia-core/tests/timemap/test_timemap_meters.py b/packages/tilia-core/tests/timemap/test_timemap_meters.py index b96be3846..89619a050 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_meters.py +++ b/packages/tilia-core/tests/timemap/test_timemap_meters.py @@ -163,9 +163,9 @@ def test_is_ignored_and_the_unit_before_carries_on(self, bad, caplog): assert [m.time_signature for m in result] == [(2, 4), (6, 8), (6, 8), (6, 8)] assert [m.starts_here for m in result] == [True, True, False, False] assert [m.beat_unit_id for m in result][1:] == [before] * 3 - (warning,) = [ - r for r in caplog.records if r.name == "tilia_core.timemap.meters" - ] + (warning,) = caplog.records + assert warning.name == "tilia_core.timemap._rows" + assert timeline.id in warning.getMessage() assert ignored in warning.getMessage() def test_before_a_readable_one_does_not_govern_its_measure(self): diff --git a/packages/tilia-core/tests/timemap/test_timemap_rows.py b/packages/tilia-core/tests/timemap/test_timemap_rows.py index 641497607..5a7d91f63 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_rows.py +++ b/packages/tilia-core/tests/timemap/test_timemap_rows.py @@ -1,3 +1,5 @@ +import pytest + from docs import beat_timeline from tilia_core import tla from tilia_core.timemap._rows import rows_of @@ -148,6 +150,42 @@ def test_a_beat_unit_that_cannot_be_read_is_ignored(caplog): assert "beat unit" in caplog.text +@pytest.mark.parametrize( + "beat_unit", + [ + {"denominator": 8, "units": "a"}, + {"denominator": 8, "units": "0"}, + {"denominator": 8, "units": "2+"}, + {"denominator": 8, "units": 3}, + {"denominator": 0, "units": "1"}, + {"denominator": 129, "units": "1"}, + {"denominator": "8", "units": "1"}, + ], +) +def test_a_beat_unit_desktop_621_refuses_is_left_out(beat_unit, caplog): + timeline = beat_timeline( + [ + {"time": 0.0, "measure": {"number": 1}}, + {"time": 1.0, "beat_unit": beat_unit}, + ] + ) + (row,) = rows_of(timeline) + + assert row.units == () + (warning,) = caplog.records + assert timeline.id in warning.getMessage() + assert row.beat_ids[1] in warning.getMessage() + + +def test_the_warning_shortens_a_long_unit(caplog): + unit = {"denominator": 8, "units": "x" * 5000} + timeline = beat_timeline([{"time": 0.0, "measure": {}, "beat_unit": unit}]) + (row,) = rows_of(timeline) + + assert row.units == () + assert len(caplog.records[0].getMessage()) < 300 + + def test_a_measure_whose_downbeat_has_no_time_is_skipped(caplog): timeline = beat_timeline( [ From 27c0a5d3aba613eee093e7b6a3d12e6390709a14 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:08:50 +0200 Subject: [PATCH 5/5] feat(core): give the measures before the first beat unit its values, assumed Before the first unit meters_of can read, the measures now take that unit's denominator and units, assumed, as desktop#621 (beat units and time signatures)'s loader gives them: it keeps a unit in the first measure (_ensure_first_beat_unit), copied from the first unit. With no unit at all, a measure of N taps is still N/4, assumed, as #621's default unit is. Either way measure 1 has starts_here true, as in TiLiA, and no beat_unit_id, since no unit sits on its beats. Tests cover a timeline whose first measures have no unit (or only one that can't be read) and a unit later, and one with no readable unit at all. Part of TimeLineAnnotator/project-management#213 Co-Authored-By: Claude Opus 5.5 --- .../src/tilia_core/timemap/meters.py | 25 +++++++--- .../tests/timemap/test_timemap_meters.py | 49 +++++++++++++++++-- 2 files changed, 62 insertions(+), 12 deletions(-) diff --git a/packages/tilia-core/src/tilia_core/timemap/meters.py b/packages/tilia-core/src/tilia_core/timemap/meters.py index 56433d0a1..123945daf 100644 --- a/packages/tilia-core/src/tilia_core/timemap/meters.py +++ b/packages/tilia-core/src/tilia_core/timemap/meters.py @@ -40,22 +40,33 @@ def meters_of(rows: Sequence[RowIn]) -> list[MeasureMeter]: The meter of every measure, from the beat units in effect: the loop of desktop#621's `BeatTimeline._compute_measure_meters`. The first unit set in a measure governs it and is carried forward; any other in that measure - makes it conflicting. Until a unit is set, a measure of N taps is N/4, - assumed. `rows_of` has already left out the units that can't be read, so - the unit before such a unit carries on. + makes it conflicting. `rows_of` has already left out the units that can't + be read, so the unit before such a unit carries on. + + Before the first unit, the measures take its denominator and units, + assumed; with no unit at all, a measure of N taps is N/4, assumed. Either + way measure 1 starts a unit, but none has a `beat_unit_id`, since no unit + sits on their beats. Only here does this differ from #621's loop, which + never meets such a timeline: #621's loader keeps a unit in the first + measure (`_ensure_first_beat_unit`), copied from the first unit or the + default. """ - default_units = parse_units(DEFAULT_UNITS).units + first = next((_parsed(*unit) for row in rows for unit in row.units), None) + if first is None: + opening = DEFAULT_DENOMINATOR, tuple(parse_units(DEFAULT_UNITS).units) + else: + opening = first.denominator, first.unit_values meters = [] governing: _BeatUnit | None = None - for row in rows: + for index, row in enumerate(rows): beat_count = len(row.beat_ids) in_measure = [_parsed(beat_id, unit) for beat_id, unit in row.units] if in_measure: governing = in_measure[0] if governing is None: - denominator, units = DEFAULT_DENOMINATOR, list(default_units) + denominator, units = opening[0], list(opening[1]) else: denominator, units = governing.denominator, list(governing.unit_values) @@ -65,7 +76,7 @@ def meters_of(rows: Sequence[RowIn]) -> list[MeasureMeter]: units=units, beat_values=fit_units(units, beat_count), beat_unit_id=governing.id if governing else None, - starts_here=bool(in_measure), + starts_here=bool(in_measure) or index == 0, is_ambiguous=is_fit_ambiguous(units, beat_count), is_conflicting=len(in_measure) > 1, is_assumed=governing is None or governing.assumed, diff --git a/packages/tilia-core/tests/timemap/test_timemap_meters.py b/packages/tilia-core/tests/timemap/test_timemap_meters.py index 89619a050..99c08b70e 100644 --- a/packages/tilia-core/tests/timemap/test_timemap_meters.py +++ b/packages/tilia-core/tests/timemap/test_timemap_meters.py @@ -117,17 +117,56 @@ def test_two_beat_units_in_a_measure_first_wins(self): assert meter.denominator == 8 -class TestDefaultAndAssumed: - def test_without_any_unit_a_measure_of_n_taps_is_n_over_4(self): - timeline = tapped(7, units={0: None}) +class TestBeforeTheFirstUnit: + # TiLiA's loader keeps a unit in the first measure: a copy of the first + # unit, assumed, or the default. No unit sits on those beats in the file. + + @pytest.mark.parametrize("first", [None, unit(8, "x")]) + def test_the_measures_take_the_first_units_values_assumed(self, first): + timeline = tapped(8, (2,), {0: first, 6: unit(8, "3")}) result = meters(timeline) - assert time_signatures(timeline) == [(4, 4), (3, 4)] + assert [m.time_signature for m in result] == [(6, 8)] * 4 + assert [m.units for m in result] == [[3]] * 4 + assert [m.is_assumed for m in result] == [True, True, True, False] + assert [m.starts_here for m in result] == [True, False, False, True] + assert [m.beat_unit_id for m in result] == [ + None, + None, + None, + beat_ids(timeline)[6], + ] + assert not any(m.is_conflicting for m in result) + + def test_a_pattern_is_read_from_its_start_in_those_measures(self): + timeline = tapped(5, (1, 2, 2), {0: None, 3: unit(8, "2+3")}) + + result = meters(timeline) + assert [m.beat_values for m in result] == [[2], [2, 3], [2, 3]] + assert [m.is_ambiguous for m in result] == [True, False, False] + + @pytest.mark.parametrize("first", [None, unit(8, "x")]) + def test_with_no_readable_unit_a_measure_of_n_taps_is_n_over_4(self, first): + timeline = tapped(7, units={0: first}) + + result = meters(timeline) + assert [m.time_signature for m in result] == [(4, 4), (3, 4)] assert [m.units for m in result] == [[1], [1]] assert [m.beat_unit_id for m in result] == [None, None] - assert [m.starts_here for m in result] == [False, False] + assert [m.starts_here for m in result] == [True, False] assert all(m.is_assumed for m in result) + def test_a_unit_in_the_first_measure_is_its_own(self): + timeline = tapped(8, units={0: None, 2: unit(8, "1")}) + + first = meters(timeline)[0] + assert first.beat_unit_id == beat_ids(timeline)[2] + assert first.starts_here + assert not first.is_assumed + assert first.time_signature == (4, 8) + + +class TestAssumed: def test_assumed_follows_the_governing_unit(self): timeline = tapped(12, units={4: unit(8, "1"), 8: unit(8, "2", assumed=True)})