feat(parsing): support parsing leap seconds by normalizing 60 to 59 seconds#289
feat(parsing): support parsing leap seconds by normalizing 60 to 59 seconds#289gtkacz wants to merge 9 commits intoariebovenberg:mainfrom
Conversation
|
@gtkacz thanks for taking the time to pick this up. Looks good so far. Let me know if you have any questions/suggestions to improve the contributor experience. The contributor instructions are a bit light on detail at the moment. |
I do think there's a couple small things that the repo could use:
Nothing too major. You can definitely do everything you need by looking around for a couple minutes but it'd be cool to have it all in the contributing docs. I can do some of these pretty easily if you want, just let me know which ones and if I should associate them to issues and etc. Keep up the great work! :) |
|
@ariebovenberg testing is behaving weird. The pure Python implementations fails on Python > 3.10. Do you have any idea what could be causing this? |
The prime suspect would be that the two The reason there's 2 functions, is to ensure Python 3.11 takes advantage of the fast edit: typo |
|
Oh duh yeah, total brainfart on my part. Sorry for that, been having a busy week. Will fix it later today |
|
So @ariebovenberg I tried running the tests on Windows/Python 3.13.0 and WSL Ubuntu/Python 3.13.5 and both are being successful. I double checked both of my implementations of |
|
@gtkacz no problem, I'll have a look what's going on |
|
Thank you! Let me know if you find anything and I'll fix it :) |
|
It does seem to be a problem with given a basic time like # _pywhenever.py:5890
if s.endswith(":60"):
s = s[:-2] + "59"Leap seconds are only triggered in extended format, because it's checked for a colon |
|
You're right. I forgot that the second implementation has an explicit check for len == 6 that short circuits the colon implementation. Still, it doesn't make sense to me that the tests were passing on my machine lol |
|
I'm AFK at the moment, hope to review this weekend |
|
No rush! :) |
ariebovenberg
left a comment
There was a problem hiding this comment.
Looks good. Only some minor changes needed.
|
|
||
| def test_leap_seconds_parsing(self): | ||
| # Leap second (60) should be parsed and normalized to 59 | ||
| assert ZonedDateTime.parse_iso( |
There was a problem hiding this comment.
Let's move these ISO-parsing test cases to the parametrized TestParseIso.test_valid test case. You can add comments above them that these are leap second test cases.
|
|
||
| def test_leap_seconds_comprehensive(self): | ||
| # Test with UTC - leap second normalization works across timezones | ||
| dt = ZonedDateTime.parse_iso("2020-08-15T12:34:60+00:00[UTC]") |
There was a problem hiding this comment.
These parse_iso test cases can also be moved to TestParseIso.test_valid
|
@ariebovenberg been having a really busy week at work, sorry. I'll get around to the PR by Friday |
|
@gtkacz no problem. Be sure to rebase once you get back, there's been an 0.9.3 release in the meantime |
Description
Many standards and libraries allow for times like 23:59:60 to represent leap seconds.
whenevernow does so by allowing 60 seconds when ingesting or parsing data, but constraining it to 59 in the actual class. Closes #90.Checklist
Release checklist (maintainers only)
pyproject.tomlsrc/whenever/__init__.py