Fix: Handle XSD end-of-day midnight notation 24:00:00#582
Fix: Handle XSD end-of-day midnight notation 24:00:00#582paul-gerber-svg wants to merge 2 commits into
Conversation
Before, parsing XSD timestamps with an hour of 24 causes a ValueError. Now, the parser in `sdk/basyx/aas/model/datatypes.py` correctly identifies 24:00:00 as midnight, handles the day rollover for `DateTime`, and normalizes `Time` to 00:00:00. Unit tests for valid and invalid edge cases are added to `sdk/test/model/test_datatypes.py` inside `TestDateTimeTypes`. Fixes eclipse-basyx#564
| is_midnight_24 = False | ||
| if hour == 24: | ||
| if int(match[6]) != 0 or int(match[7]) != 0 or microseconds != 0: | ||
| raise ValueError("Invalid time: 24:00:00.000000 is the only valid representation of midnight") |
There was a problem hiding this comment.
I think this error message may be confusing.
Technically 00:00:00 is also a valid reüresentation of midnight. I think I'd keep it more general, something like;
| raise ValueError("Invalid time: 24:00:00.000000 is the only valid representation of midnight") | |
| raise ValueError(f"{value} is not a valid xsd:datetime.") |
| res = DateTime(int(match[2]), int(match[3]), int(match[4]), hour, int(match[6]), int(match[7]), | ||
| microseconds, _parse_xsd_date_tzinfo(match[9])) |
There was a problem hiding this comment.
Would it be possible to use keyword arguments here? I don't know if DateTime supports this, but that's pretty hard to verify. I mean something like:
DateTime(
year=int(match[2]),
month=int(match[3]),
day=int(match[4]),
hour=hour,
...
)| microseconds = int(float(match[8]) * 1e6) if match[8] else 0 | ||
| return DateTime(int(match[2]), int(match[3]), int(match[4]), int(match[5]), int(match[6]), int(match[7]), | ||
| microseconds, _parse_xsd_date_tzinfo(match[9])) | ||
| hour = int(match[5]) |
There was a problem hiding this comment.
Can you add a comment above, linking to the issue (#564) and explain why this code exists, something like:
xsd_datetime allows for hour=24 to represent midnight, Python's datetime.DateTime doesn't.
If we get an hour=24, we accept and parse it as hour=0 of the next day.
| return res + datetime.timedelta(days=1) if is_midnight_24 else res | ||
|
|
||
|
|
||
| def _parse_xsd_time(value: str) -> Time: |
There was a problem hiding this comment.
Same comments as for _parse_xsd_datetime
2 participants
Before, parsing XSD timestamps with an hour of 24 causes a ValueError. Now, the parser in
sdk/basyx/aas/model/datatypes.pycorrectly identifies 24:00:00 as midnight, handles the day rollover forDateTime, and normalizesTimeto 00:00:00.Unit tests for valid and invalid edge cases are added to
sdk/test/model/test_datatypes.pyinsideTestDateTimeTypes.Fixes #564