From 0aa02bce6de59a90e0228731fe20a355583b3d4d Mon Sep 17 00:00:00 2001 From: Tabish Mustufa Date: Sat, 5 Sep 2026 17:56:31 -0700 Subject: [PATCH] Stop deriving Activity fields for a file holding more than one activity Merged records/laps from a chained FIT file, multi-Activity TCX, or multi-track GPX can't be attributed to just the first activity, so those fields are left None instead of a mixed aggregate. Co-Authored-By: Claude Sonnet 5 --- src/activity_parser/activity.py | 3 +++ src/activity_parser/parse_fit.py | 3 ++- src/activity_parser/parse_tcx_gpx.py | 8 ++++++-- tests/test_parse_fit.py | 3 +++ tests/test_parse_tcx_gpx.py | 10 +++++++++- tests/test_readme.py | 2 +- 6 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/activity_parser/activity.py b/src/activity_parser/activity.py index 4280f19..aa518da 100644 --- a/src/activity_parser/activity.py +++ b/src/activity_parser/activity.py @@ -94,6 +94,9 @@ def fill_activity(activity: Activity, records: pd.DataFrame, laps: pd.DataFrame) A field the file itself reports is never overwritten; this only fills fields the file left ``None``. ``total_timer_time``, ``total_ascent`` and ``total_descent`` are not derived and are left as the file reported them, ``None`` included. + + Should not be called for a file holding more than one activity, since the filled + fields would aggregate over all activities. """ # Prefer sum of lap data, fall back to calculating from records total_elapsed_time = _coalesce( diff --git a/src/activity_parser/parse_fit.py b/src/activity_parser/parse_fit.py index 9bd0d05..bcb52c7 100644 --- a/src/activity_parser/parse_fit.py +++ b/src/activity_parser/parse_fit.py @@ -314,7 +314,8 @@ def parse_fit( ) activity = build_activity(convert_units_mapping(session or {}, SESSION_UNITS), file_id) - activity = fill_activity(activity, records, laps) + if len(messages.get("session", [])) <= 1: + activity = fill_activity(activity, records, laps) return records, laps, activity diff --git a/src/activity_parser/parse_tcx_gpx.py b/src/activity_parser/parse_tcx_gpx.py index db00791..fee9049 100644 --- a/src/activity_parser/parse_tcx_gpx.py +++ b/src/activity_parser/parse_tcx_gpx.py @@ -221,7 +221,9 @@ def parse_tcx( etree.strip_elements(root, "{*}Track") laps = build_dataframe(root.iter("{*}Lap"), TCX_LAP_FIELDS) - activity = fill_activity(tcx_activity(root), records, laps) + activity = tcx_activity(root) + if len(root.findall(".//{*}Activity")) <= 1: + activity = fill_activity(activity, records, laps) return records, laps, activity @@ -261,6 +263,8 @@ def parse_gpx( records = build_dataframe(root.iter("{*}trkpt"), GPX_TRACKPOINT_FIELDS) records = index_by_time(records, "time") - activity = fill_activity(gpx_activity(root), records, pd.DataFrame()) + activity = gpx_activity(root) + if len(root.findall("{*}trk")) <= 1: + activity = fill_activity(activity, records, pd.DataFrame()) return records, pd.DataFrame(), activity diff --git a/tests/test_parse_fit.py b/tests/test_parse_fit.py index 15c1dc4..181f053 100644 --- a/tests/test_parse_fit.py +++ b/tests/test_parse_fit.py @@ -402,6 +402,9 @@ def test_parse_fit_multi_session_uses_first_session_and_file_id(): assert activity.sport == "running" assert activity.total_elapsed_time == 60.0 assert activity.creator == "garmin 1111" + # Nothing from the second session leaks in. + assert activity.total_distance is None + assert activity.avg_heart_rate is None def test_parse_fit_chained_files(tmp_path): diff --git a/tests/test_parse_tcx_gpx.py b/tests/test_parse_tcx_gpx.py index a1cde47..cea4787 100644 --- a/tests/test_parse_tcx_gpx.py +++ b/tests/test_parse_tcx_gpx.py @@ -227,6 +227,12 @@ def test_tcx_multi_activity_merges_records_and_laps_first_activity_wins_summary( assert laps["total_distance"].tolist() == pytest.approx([0.5, 1.0]) assert activity.sport == "cycling" assert activity.start_time == pd.Timestamp("2026-01-05T08:00:00Z") + # Computed fields must not mix the two Activities' merged records/laps together. + assert activity.total_elapsed_time is None + assert activity.total_distance is None + assert activity.avg_heart_rate is None + assert activity.max_heart_rate is None + assert activity.avg_speed is None # --------------------------------------------------------------------------- @@ -401,8 +407,10 @@ def test_gpx_1_0_base_fields(): def test_gpx_multiple_tracks_merged(): # Two separate elements, unlike multi_segment.gpx's two in one. - records, _, _ = ActivityParser().parse(MULTI_TRACK_GPX) + records, _, activity = ActivityParser().parse(MULTI_TRACK_GPX) assert records["latitude"].tolist() == pytest.approx([37.0, 37.5]) + # Not derived from the merged records' time span. + assert activity.total_elapsed_time is None # --------------------------------------------------------------------------- diff --git a/tests/test_readme.py b/tests/test_readme.py index f4ed26a..f4743d0 100644 --- a/tests/test_readme.py +++ b/tests/test_readme.py @@ -41,5 +41,5 @@ def test_readme_laps_table_matches_default_columns(): def test_readme_activity_table_matches_dataclass_fields(): text = README.read_text() - section = _section(text, "### Activity", "## Parser notes") + section = _section(text, "### Activity", "## Examples") assert _table_column_names(section) == [f.name for f in fields(Activity)]