-
Notifications
You must be signed in to change notification settings - Fork 94
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
Review iso8601.py
code comments (and code?)
#4805
Comments
Maybe the authors of those comments will remember. The first were by @lhuggett ; the second by @oliver-sanders . |
So I think what the first one was about was that if you include the year (and/or month) from the real "now" when you're not looking at a day as well, you end up with a false earliness, so you move it (them) forward one to make sure the next/previous works correctly. I do recall that I only found it by trial and error. For the second one, I'm afraid I don't remember what the issue was about the potential breakage from the reversing of tickover order, only that it was something that we thought could be a problem but hadn't actually been demonstrated as one at that point. |
Thanks for the response, looks like the first one comes down to a possible bug in isodatetime: >>> now = TimePointParser().parse("2010-08-08T15:40Z")
>>> point = TimePointParser(allow_truncated=True).parse("-10")
>>> now + point
<metomi.isodatetime.data.TimePoint: 2010-08-08T15:40:00Z>
>>> now == now + point
True I think Possibly related to metomi/isodatetime#80 |
For the second, I think I have figured it out: metomi/isodatetime#212 |
There are several cryptic comments such as
cylc-flow/cylc/flow/cycling/iso8601.py
Lines 669 to 677 in 5a19433
cylc-flow/cylc/flow/cycling/iso8601.py
Lines 771 to 782 in 5a19433
In the latter, unfortunately the linked PR metomi/isodatetime#101 is not very illuminating to me! (And the discussion there appears as if it might be suffering from confusion over the "addition of truncated TimePoint" bug metomi/isodatetime#80, to boot)
Also, is it just me or are the regexes in the code alarmingly simple? We should see if we can find faults in them.
The text was updated successfully, but these errors were encountered: