Skip to content
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

Update to isodatetime 3.0 #4554

Merged
merged 8 commits into from
Apr 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,16 @@ ones in. -->
-------------------------------------------------------------------------------
## __cylc-8.0rc3 (<span actions:bind='release-date'>Pending</span>)__

### Fixes:
Third Release Candidate for Cylc 8 suitable for acceptance testing.

### Fixes

[#4554](https://github.com/cylc/cylc-flow/pull/4554) - Fix incorrect
implementation of the ISO 8601 recurrence format no. 1
(`R<number>/<start-point>/<second-point>`)
(see [metomi/isodatetime#45](https://github.com/metomi/isodatetime/issues/45)).
This recurrence format was not mentioned in the Cylc documentation, so
this is unlikely to affect you.

[#4797](https://github.com/cylc/cylc-flow/pull/4797) -
`cylc reload` now triggers a fresh remote file installation for all relevant
Expand Down
2 changes: 1 addition & 1 deletion conda-environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the upper pin is a good idea for isodatetime!

- protobuf >=3.19.0,<3.20.0
- psutil >=5.6.0
- python
Expand Down
52 changes: 26 additions & 26 deletions cylc/flow/cycling/iso8601.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import re
from typing import List, Optional, TYPE_CHECKING, Tuple

from metomi.isodatetime.data import Calendar, CALENDAR
from metomi.isodatetime.data import Calendar, CALENDAR, Duration
from metomi.isodatetime.dumpers import TimePointDumper
from metomi.isodatetime.timezone import (
get_local_time_zone, get_local_time_zone_format, TimeZoneFormatMode)
Expand Down Expand Up @@ -66,7 +66,7 @@ class WorkflowSpecifics:
point_parser: 'TimePointParser' = None
recurrence_parser: 'TimeRecurrenceParser' = None
iso8601_parsers: Optional[
Tuple['DurationParser', 'TimePointParser', 'TimeRecurrenceParser']
Tuple['TimePointParser', 'DurationParser', 'TimeRecurrenceParser']
] = None


Expand Down Expand Up @@ -383,10 +383,7 @@ def get_offset(self):

def set_offset(self, i_offset):
"""Deprecated: alter state to i_offset the entire sequence."""
if self.recurrence.start_point is not None:
self.recurrence.start_point += interval_parse(str(i_offset))
if self.recurrence.end_point is not None:
self.recurrence.end_point += interval_parse(str(i_offset))
self.recurrence += interval_parse(str(i_offset))
self._cached_first_point_values = {}
self._cached_next_point_values = {}
self._cached_valid_point_booleans = {}
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this still be here? If yes, is there a matching issue?

Copy link
Member Author

@MetRonnie MetRonnie Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

#2815 (comment)

Sadly not done for this particular comment 😬

Copy link
Member Author

@MetRonnie MetRonnie Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have opened 2 issues:

I haven't opened anything regarding --icp, would you like to do that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

if re.search(r"\(-\d{2}[);T]", value):
now.year += 1
now += Duration(years=1)

# correct for month in 'now' if year and month only,
# or year, month and time, specified in input
elif re.search(r"\(-\d{4}[);T]", value):
now.month_of_year += 1
now += Duration(months=1)

# perform whatever transformation is required
offset = None
Expand All @@ -692,13 +690,13 @@ def ingest_time(value: str, now: Optional['TimePoint'] = None) -> str:

offset = offset.replace('+', '')
offset = duration_parser.parse(offset)
cycle_point = cycle_point + offset
cycle_point += offset

return str(cycle_point)


def prev_next(
value: str, now: 'TimePoint', parser: 'TimePointParser'
value: str, now: 'TimePoint', parser: 'TimePointParser'
) -> Tuple['TimePoint', Optional[str]]:
"""Handle prev() and next() syntax.

Expand All @@ -722,10 +720,7 @@ def prev_next(
offset: Optional[str]
tmp, offset = tmp.split(")")

if offset.strip() == '':
offset = None
else:
offset = offset.strip()
offset = offset.strip() or None

str_points: List[str] = tmp.split(";")
timepoints: List['TimePoint'] = []
Expand Down Expand Up @@ -765,21 +760,28 @@ def prev_next(
# ensure truncated dates do not have
# time from 'now' included'
if 'T' not in value.split(')')[0]:
cycle_point.hour_of_day = 0
cycle_point.minute_of_hour = 0
cycle_point.second_of_minute = 0
# NOTE: Strictly speaking we shouldn't forcefully mutate TimePoints
# in this way as they're meant to be immutable since
# https://github.com/metomi/isodatetime/pull/165, however it
# should be ok as long as the TimePoint is not used as a dict key and
# we don't call any of the TimePoint's cached methods until after we've
# finished mutating it.
cycle_point._hour_of_day = 0
cycle_point._minute_of_hour = 0
cycle_point._second_of_minute = 0

# ensure month and day from 'now' are not included
# where they did not appear in the truncated datetime
# NOTE: this may break when the order of tick over
# for time point is reversed!!!
# https://github.com/metomi/isodatetime/pull/101
# case 1 - year only
if re.search(r"\(-\d{2}[);T]", value):
cycle_point.month_of_year = 1
cycle_point.day_of_month = 1
cycle_point._month_of_year = 1
cycle_point._day_of_month = 1
# case 2 - month only or year and month
elif re.search(r"\(-(-\d{2}|\d{4})[;T)]", value):
cycle_point.day_of_month = 1
cycle_point._day_of_month = 1

return cycle_point, offset

Expand Down Expand Up @@ -871,14 +873,12 @@ def get_point_relative(offset_string, base_point):
def interval_parse(interval_string):
"""Parse an interval_string into a proper Duration class."""
try:
return _interval_parse(interval_string).copy()
return _interval_parse(interval_string)
except Exception:
try:
return -1 * _interval_parse(
interval_string.replace("-", "", 1)).copy()
return -1 * _interval_parse(interval_string.replace("-", "", 1))
except Exception:
return _interval_parse(
interval_string.replace("+", "", 1)).copy()
return _interval_parse(interval_string.replace("+", "", 1))


def is_offset_absolute(offset_string):
Expand All @@ -899,7 +899,7 @@ def _interval_parse(interval_string):

def point_parse(point_string):
"""Parse a point_string into a proper TimePoint object."""
return _point_parse(point_string).copy()
return _point_parse(point_string)


@lru_cache(10000)
Expand Down
2 changes: 1 addition & 1 deletion cylc/flow/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -923,7 +923,7 @@ def command_stop(
# schedule shutdown after wallclock time passes provided time
parser = TimePointParser()
self.set_stop_clock(
int(parser.parse(clock_time).get("seconds_since_unix_epoch"))
int(parser.parse(clock_time).seconds_since_unix_epoch)
)
elif task:
# schedule shutdown after task succeeds
Expand Down
5 changes: 2 additions & 3 deletions cylc/flow/task_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,7 @@ def get_point_as_seconds(self):
"""Compute and store my cycle point as seconds since epoch."""
if self.point_as_seconds is None:
iso_timepoint = point_parse(str(self.point))
self.point_as_seconds = int(iso_timepoint.get(
'seconds_since_unix_epoch'))
self.point_as_seconds = int(iso_timepoint.seconds_since_unix_epoch)
if iso_timepoint.time_zone.unknown:
utc_offset_hours, utc_offset_minutes = (
get_local_time_zone())
Expand All @@ -324,7 +323,7 @@ def get_clock_trigger_time(self, offset_str):
else:
trigger_time = self.point + ISO8601Interval(offset_str)
self.clock_trigger_time = int(
point_parse(str(trigger_time)).get('seconds_since_unix_epoch')
point_parse(str(trigger_time)).seconds_since_unix_epoch
)
return self.clock_trigger_time

Expand Down
Loading