Refactor date/time parsing for mitpe events #1840
Conversation
mbertrand
added
Needs Review
| return dt.astimezone(UTC) | ||
|
|
||
|
|
||
| def parse_date_time_range( |
There was a problem hiding this comment.
This does not seem to get the correct time 100% of the time. For example
start_date_str="2020-01-28"
end_date_str="2020-01-28"
time_range_str="2-3:00pm"
returns start_date = datetime.datetime(2020, 1, 28, 2, 0)
end_date = datetime.datetime(2020, 1, 28, 15, 0)
before the conversion to utc
Also some events just have a start date like "1pm" and the time isn't parsed at all in that case
There was a problem hiding this comment.
Some events also have time strings that are going to be really hard to parse like "8:00pm; doors open at 7:30pm" I wonder if it makes sense to give up, store event dates then store event time as a string so it's displayed the way it was in the original listing
There was a problem hiding this comment.
Maybe I was trying too hard to get the times right; what's most important is really the start date because that's the only thing that's actually shown on the frontend. Next most important is end_date, because both are used to determine if the event should be published (start date or end date should be < now). If the professional ed api is sending back dates like "1 pm", that's just bad data and on them to fix.
How about I make sure the dates are correctly parsed (assuming the data is good), then for times, if it can't figure it out, it will just default to noon? That way when the pipeline runs in the early AM, it will keep events happening that day published for display.
news_events/etl/utils.py
Outdated
| """ | ||
| start_time, start_ampm, end_time, end_ampm, tz = "", "", "", "", "" | ||
| time_regex = re.compile( | ||
| r"(\d{1,2})(:\d{2})?\s*(am|pm)?\s*-?\s*(\d{1,2})(:?\d{2})?\s*(am|pm)?\s*([A-Za-z]{2,3})?", |
There was a problem hiding this comment.
The regex is really complicated. Could it be broken up into multiple steps? Like one step to break the time string into two in the case where there is both a start and end date and a seperate step to extract pm/am. As it is i think it's hard to troubleshoot or understand what the regex is doing
There was a problem hiding this comment.
Split up the regex and also split up the function. Added more inline comments and more unit test cases
mbertrand
force-pushed
the
mb/event_time_fix
branch
from
5957321 to
58272e8
Compare
… and convert to utc if times cannot be parsed
mbertrand
force-pushed
the
mb/event_time_fix
branch
from
58272e8 to
d23473d
Compare
What are the relevant tickets?
Closes https://github.com/mitodl/hq/issues/6114
Description (What does it do?)
Refactors the functions for mitpe event date/time parsing so they better handle the formats in the mitpe events api.
How can this be tested?
./manage.py backpopulate_news_events --pipelines mitpe_events_etlon the main branch, you will see lots of errors about the date/time being unparseable.