Update to isodatetime 3.0 #4554
Conversation
MetRonnie
force-pushed
the
update-isodatetime
branch
from
d9cae05
to
3ae2209
Compare
MetRonnie
force-pushed
the
update-isodatetime
branch
3 times, most recently
from
73cc84e
to
b92485a
Compare
| @@ -9,7 +9,7 @@ dependencies: | |||
| - graphene >=2.1,<3 | |||
| - graphviz # for static graphing | |||
| - jinja2 ==2.11.0,<2.12 | |||
| - metomi-isodatetime >=1!2.0.2, <1!2.1.0 | |||
| - metomi-isodatetime >=1!3.0.0, <1!3.1.0 | |||
There was a problem hiding this comment.
I think the upper pin is a good idea for isodatetime!
Now the data classes are immutable
MetRonnie
force-pushed
the
update-isodatetime
branch
from
b92485a
to
85e0c08
Compare
|
|
||
| run_fail "${TEST_NAME_BASE}" cylc validate . | ||
| cmp_ok "${TEST_NAME_BASE}.stderr" <<'__ERR__' | ||
| SequenceDegenerateError: R/20100101T0000Z/P0Y, point format CCYYMMDDThhmmZ: equal adjacent points: 20100101T0000Z => 20100101T0000Z. |
There was a problem hiding this comment.
This error was only raised on validation, was never raised on run. I have replaced it with a warning, see tests/unit/test_config.py
(Note the diff here should really be deletion of the file and creation of a new, unrelated test, though Git thinks it has been renamed and changed)
|
(Test failures due to usual integration tests hanging on GH Actions) |
| @@ -668,13 +665,14 @@ def ingest_time(value: str, now: Optional['TimePoint'] = None) -> str: | |||
|
|
|||
| # correct for year in 'now' if year only, | |||
| # or year and time, specified in input | |||
| # TODO: Figure out why this correction is needed | |||
There was a problem hiding this comment.
Should this still be here? If yes, is there a matching issue?
There was a problem hiding this comment.
Not sure if it's worth an issue, why ever it's doing that it seems to be working. But if someone can figure this out down the line it would be good to update the comment above to improve the explanation
There was a problem hiding this comment.
I've spent far too long trying to figure this out - I think I'd quite like an issue raised to poke this - I'm not thrilled that I've not been able to get the bottom of it.
I'm also not clear that I can find definitive docs of what --icp can accept.
There was a problem hiding this comment.
oliver-sanders on 16 Nov 2018
Comments like this have a habit of becoming a bit cryptic to future developers. Perhaps copy in the URL of the isodatetime issue.
Sadly not done for this particular comment 😬
There was a problem hiding this comment.
I have opened 2 issues:
I haven't opened anything regarding --icp, would you like to do that?
There was a problem hiding this comment.
I'm also not clear that I can find definitive docs of what --icp can accept.
This. Yes. #4807 (Put in cylc-flow not cylc-doc because although it should touch both I think the CLI help is upstream)
| @@ -779,7 +779,7 @@ mock_smtpd_kill() { # Logic borrowed from Rose | |||
| install_and_validate() { | |||
| # First part of the reftest function, to allow separate use. | |||
| # Expect 1 OK test. | |||
| local TEST_NAME="${1:-${TEST_NAME_BASE}}-validate" | |||
| local TEST_NAME="${1:-${TEST_NAME_BASE}}" | |||
There was a problem hiding this comment.
It was resulting in NN-testname-validate-validate
Pity grumpy face isn't available... But it looks ok now. |
These changes address #4110 (comment). When we release isodatetime 3.0.0, this PR allows Cylc to work with the changed API.
isodatetime 3.0.0 fixes a longstanding mistake in the implementation of recurrence format number 1 (metomi/isodatetime#45).
See also isodatetime changelog
Edit: as recurrence format 1 is undocumented (in Cylc docs) and rarely used, will postpone this to 8.0rc3
Requirements check-list
CONTRIBUTING.mdand added my name as a Code Contributor.setup.cfgandconda-environment.yml.