Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix timeslot parsing #82

Merged
merged 4 commits into from
Nov 30, 2024
Merged

Fix timeslot parsing #82

merged 4 commits into from
Nov 30, 2024

Conversation

psvenk
Copy link
Member

@psvenk psvenk commented Nov 28, 2024

Fixes #79. Properly account for timings that begin in the morning and end in the evening by special-casing these rather than treating evening times as morning times. Add assertions and give more detailed error messages.

This causes 15.089 ("W EVE (11-6 PM)") to be parsed correctly (as 11:00 AM to 6:00 PM) but also results in early morning ROTC classes (such as NS.100) throwing errors since we don't support times before 8:00 AM (these classes were previously misidentified as evening classes).

It would also make sense to overhaul the timeslot system so that early morning classes can be supported, but I'll leave that to another pull request.

Properly account for timings that begin in the morning and end in the
evening by special-casing these rather than treating evening times as
morning times. Add assertions and give more detailed error messages.

This causes 15.089 ("W EVE (11-6 PM)") to be parsed correctly but also
results in early morning ROTC classes (such as NS.100) throwing errors
since we don't support times before 8:00 AM (these classes were
previously misidentified as evening classes).
scrapers/fireroad.py Show resolved Hide resolved
scrapers/fireroad.py Show resolved Hide resolved
scrapers/fireroad.py Outdated Show resolved Hide resolved
@psvenk psvenk requested a review from dtemkin1 November 30, 2024 20:40
Copy link
Collaborator

@dtemkin1 dtemkin1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@psvenk psvenk merged commit 83801ed into main Nov 30, 2024
3 checks passed
@psvenk psvenk deleted the timeslots branch November 30, 2024 21:00
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.

fireroad.py can't parse schedule for 15.089?
3 participants