Handle military clock time (0800) in time standardizer. #1056
Conversation
| ("2023-07-15 10.30.00", "2023-07-15 10:30:00"), | ||
| ("15/07/2023 10.30.00", "2023-07-15 10:30:00"), | ||
| ("2023-07-15 10.30.00 Local", "2023-07-15 10:30:00"), | ||
| ("2023-07-15 10.30.00PDT", "2023-07-15 10:30:00-07:00"), |
There was a problem hiding this comment.
Why do we need to preserve the timezone? Why not convert to UTC time? That seems far easier to parse and a better basis especially when others don't have any timezone mentioned.
There was a problem hiding this comment.
It's not my decision to make. I assume the reason is that sometimes the timezone isn't specified in either the document or the query and it's better to be in a position to do late binding. The way datetime and dateparser work is that timezone can be specified if known and will be used if specified. It's easy to convert a datetime to UTC if that's the goal.
There was a problem hiding this comment.
But if we want to 'standardize' it, isn't standardizing to one timezone part of the process? It seems like that's likely to reduce errors as well in that case. Also, how does OpenSearch do with timezones?
| DEFAULT_FORMAT = "%B %d, %Y %H:%M:%S%Z" | ||
| clock_re = re.compile(r"\d:[0-5]\d") | ||
| year_re = re.compile(r"([12]\d\d\d-)|(/[12]\d\d\d)|(\d/[0-3]?\d/\d)") | ||
| digitpair_re = re.compile(r"([0-2]\d)([0-5]\d)(\d\d)?") |
There was a problem hiding this comment.
Would appreciate comments for the regexes
There was a problem hiding this comment.
I had hoped the names were descriptive. Do you want examples of what they match? Not sure how to make regexes clearer without just explaining the grammar of regexes, which is already better done on various web pages.
There was a problem hiding this comment.
Examples would be good, or even a short explanation of their functionality. It is very dense right now, and not easy to understand without trying to make sense of the regex itself.
| raw_dateTime = raw_dateTime.replace("Local", "") | ||
| raw_dateTime = raw_dateTime.replace("local", "") | ||
| raw_dateTime = raw_dateTime.replace(".", ":") | ||
| logging.error(f"FIXME {raw_dateTime}") |
| raise RuntimeError(f"Unexpected error occurred while processing: {raw_dateTime}") from e | ||
|
|
||
| @staticmethod | ||
| def preprocess(raw: str) -> str: |
There was a problem hiding this comment.
This looks like it's only meant for military time, so I'd rather rename this to miltary_time_preprocess, given that we're doing some processing in the main function as well.
There was a problem hiding this comment.
Fine. I was thinking other preprocessing might go here too. Either way.
There was a problem hiding this comment.
We can always rename it at that point.
No description provided.