Skip to content

Fix scheduled stop times at 24:00 and later when building bus_stop_time - #32

Open
eastagiletracker wants to merge 1 commit into
mansueto-institute:mainfrom
eastagiletracker:agile-board/gtfs-times-past-midnight
Open

eastagiletracker wants to merge 1 commit into
mansueto-institute:mainfrom
eastagiletracker:agile-board/gtfs-times-past-midnight

Conversation

@eastagiletracker

Copy link
Copy Markdown

This PR proposes a fix for the scheduled stop times at 24:00:00 and later that dedupe_schedules writes into data/clean_timetables, so hours past midnight roll into the service date instead of being rewritten in place (Fixes #21). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/500. You can sign in with your GitHub ID to claim ownership of the project.

What happens today

report_automation/update_schedule.py builds bus_stop_time from the GTFS service date and arrival_time, rewriting the hour in place for times that start with 24:

current["time_edit"] = np.where(
    current["arrival_time"].str.slice(0, 2) == "24",
    current["arrival_time"].str.replace("24", "00"),
    current["arrival_time"],
)

Series.str.replace defaults to regex=False and replaces every occurrence rather than just the hour, so a value whose minutes or seconds are also 24 loses them. Separately, 25:XX:XX never matches the == "24" test at all, so it reaches pd.to_datetime(..., errors="coerce") as an unparseable string and lands as NaT.

Running that expression verbatim at 7a4e554 on main, over arrival_time values taken from the current CTA google_transit.zip:

arrival_time   bus_stop_time        expected
06:24:00       2026-08-10 06:24:00  2026-08-10 06:24:00   ok
23:59:30       2026-08-10 23:59:30  2026-08-10 23:59:30   ok
24:06:24       2026-08-11 00:06:00  2026-08-11 00:06:24   wrong
24:24:00       2026-08-11 00:00:00  2026-08-11 00:24:00   wrong
24:32:24       2026-08-11 00:32:00  2026-08-11 00:32:24   wrong
25:08:00       NaT                  2026-08-11 01:08:00   dropped

In that feed (fetched 2026-08-06) 50,938 stop_times rows carry an hour of 24. Of those, 1,685 also have 24 in the minutes or the seconds and silently lose it — 24:24:00 becomes midnight rather than 00:24. A further 728 rows at hour 25 become NaT. The NaT half is already worked around twice in the repo: the comment you left at the assignment, and the is_not_null filter in metrics_utils.create_trips_df that drops schedule rows with no bus_stop_time. The minutes-and-seconds half is not guarded anywhere, because a mangled value is still a perfectly well-formed timestamp.

On blast radius, so the tradeoff is clear rather than oversold: stop_metrics.time_to_next_stop defaults to is_daytime=True and keeps only hours 6 to 20, so none of these rows reach the published metrics as things stand. What does change is data/clean_timetables/*.parquet itself. It is written with date and arrival_time dropped, so bus_stop_time is the only surviving time value — the wrong times are permanent in the archive you publish and push to S3, and cannot be reconstructed later. That is also what makes the late-night window unusable the moment is_daytime is turned off.

The change

build_bus_stop_time adds the stop time to the service date as an offset instead of editing the hour text:

return pd.to_datetime(dates, format="%Y%m%d") + pd.to_timedelta(times, errors="coerce")

pd.to_timedelta already understands GTFS hours past 24, so 24:06:24 and 25:08:00 roll into the next day with their minutes and seconds intact, ordinary times are untouched, and missing or unparseable values still become NaT exactly as before. The time_edit and date_edit columns are now dead, so they come out of the drop list, and the numpy import goes with them since np.where was its only use in the module.

I left the is_not_null filter in metrics_utils.create_trips_df alone. It no longer has anything to drop from this cause, but it is still a reasonable guard and removing it would change behavior you may want for other reasons.

Verification

cta-stop-watch/report_automation/test_update_schedule.py is new, stdlib unittest, no added dependency. From report_automation/:

python -m unittest test_update_schedule

Five tests cover ordinary times, the hour-24 rollover, the hour-25 rollover, unusable values, and the returned dtype. Against the previous expression the hour-24 and hour-25 tests fail while the three controls stay green; against this change all five pass.

I also ran dedupe_schedules() itself end to end, over a small rt21 timetable fixture plus an existing historic parquet, on both trees:

arrival_time   before               after
24:06:24       2026-08-11 00:06:00  2026-08-11 00:06:24
24:24:00       2026-08-11 00:00:00  2026-08-11 00:24:00
25:08:00       NaT                  2026-08-11 01:08:00
               2 null of 8 rows     0 null of 8 rows

The historic row outside the current window survived on both sides and the column drop still succeeds, so the merge with data/clean_timetables is unchanged. The repo ships no test or CI configuration, so for a before/after comparison I used compileall plus an import of every module in report_automation: identical on both trees, including update_metrics failing its memory_profiler import the same way each time. That one is unrelated to this change — memory_profiler is imported but not declared in pyproject.toml — so I left it alone rather than folding it in here.

How this was managed

We imported this repository's issues and pull requests onto a live agile board — 31 stories — and used it to manage this work. The story this PR delivers is fixing time in schedule past 1am, and the board it sits on is at https://eastagiletracker.com/projects/500.

board

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.


Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com

Schedule times of 24:00:00 and later belong to the previous service day.
dedupe_schedules rewrote the hour in place with a substring replace, which
also rewrote a minute or second of "24" in the same value, and left hours of
25 and above unparseable so they were coerced to NaT.

Build bus_stop_time by adding the stop time to the service date instead, so
the extra hours roll into the date and the rest of the value is untouched.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant