From f5885ad2e26c6fbf9544ee62c26c62abf35c6e4a Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Thu, 10 Sep 2020 10:57:13 +0100 Subject: [PATCH 1/8] Update to isodatetime 3.0.0 Now the data classes are immutable --- conda-environment.yml | 2 +- cylc/flow/cycling/iso8601.py | 43 +++++++++++++++++++----------------- cylc/flow/scheduler.py | 2 +- cylc/flow/task_proxy.py | 5 ++--- cylc/flow/time_parser.py | 4 ++-- setup.cfg | 2 +- 6 files changed, 30 insertions(+), 28 deletions(-) diff --git a/conda-environment.yml b/conda-environment.yml index b25eb33e867..85d5308cd2d 100644 --- a/conda-environment.yml +++ b/conda-environment.yml @@ -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 - protobuf >=3.19.0,<3.20.0 - psutil >=5.6.0 - python diff --git a/cylc/flow/cycling/iso8601.py b/cylc/flow/cycling/iso8601.py index 2dff5a95b4d..d47214b456f 100644 --- a/cylc/flow/cycling/iso8601.py +++ b/cylc/flow/cycling/iso8601.py @@ -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) @@ -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 = {} @@ -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 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 @@ -692,7 +690,7 @@ 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) @@ -765,9 +763,16 @@ 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 + 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 we don't call any of my_cp's methods + # (specifically, the ones that get cached) until after we've + # finished mutating it. I think. + # 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 @@ -775,11 +780,11 @@ def prev_next( # 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 @@ -871,14 +876,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): @@ -899,7 +902,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) diff --git a/cylc/flow/scheduler.py b/cylc/flow/scheduler.py index b3b620d6ee4..f7f346fc3d8 100644 --- a/cylc/flow/scheduler.py +++ b/cylc/flow/scheduler.py @@ -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 diff --git a/cylc/flow/task_proxy.py b/cylc/flow/task_proxy.py index e054405ad75..d9768e903f9 100644 --- a/cylc/flow/task_proxy.py +++ b/cylc/flow/task_proxy.py @@ -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()) @@ -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 diff --git a/cylc/flow/time_parser.py b/cylc/flow/time_parser.py index a57d717a716..2f657cc5417 100644 --- a/cylc/flow/time_parser.py +++ b/cylc/flow/time_parser.py @@ -340,7 +340,7 @@ def _get_point_from_expression(self, expr, context, is_required=False, raise CylcMissingContextPointError( "Missing context cycle point." ) - return context.copy(), None + return context, None return None, None expr_point = None expr_offset = None @@ -365,7 +365,7 @@ def _get_point_from_expression(self, expr, context, is_required=False, else: expr_offset = expr_offset + expr_offset_item if not expr and allow_truncated: - return context.copy(), expr_offset + return context, expr_offset for invalid_rec, msg in self.POINT_INVALID_FOR_CYLC_REGEXES: if invalid_rec.search(expr): raise CylcTimeSyntaxError("'%s': %s" % (expr, msg)) diff --git a/setup.cfg b/setup.cfg index 4df02acb2d5..e32eb114ba9 100644 --- a/setup.cfg +++ b/setup.cfg @@ -65,7 +65,7 @@ install_requires = colorama>=0.4,<=1 graphene>=2.1,<3 jinja2==2.11.* - metomi-isodatetime>=1!2.0.2, <1!2.1.0 + metomi-isodatetime==1!3.0.* protobuf==3.19.* psutil>=5.6.0 pyuv==1.4.* From 0628ec87b49c4586640c5976c1d94af1308a55cc Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Mon, 1 Nov 2021 18:36:53 +0000 Subject: [PATCH 2/8] Fix mistake in type annotation --- cylc/flow/cycling/iso8601.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cylc/flow/cycling/iso8601.py b/cylc/flow/cycling/iso8601.py index d47214b456f..3a8872fb26d 100644 --- a/cylc/flow/cycling/iso8601.py +++ b/cylc/flow/cycling/iso8601.py @@ -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 From a27d0a3ff3afbd381e2e0f27fac00509499f50ff Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Mon, 1 Nov 2021 18:37:01 +0000 Subject: [PATCH 3/8] Type annotations --- cylc/flow/time_parser.py | 73 ++++++++++++++++++++++++++-------------- 1 file changed, 48 insertions(+), 25 deletions(-) diff --git a/cylc/flow/time_parser.py b/cylc/flow/time_parser.py index 2f657cc5417..8bc39f8c20f 100644 --- a/cylc/flow/time_parser.py +++ b/cylc/flow/time_parser.py @@ -30,15 +30,24 @@ """ import re +from typing import TYPE_CHECKING, List, Optional, Tuple, Union -import metomi.isodatetime.data -import metomi.isodatetime.parsers +from metomi.isodatetime.data import Duration, TimeRecurrence from metomi.isodatetime.exceptions import IsodatetimeError +from metomi.isodatetime.parsers import ( + DurationParser, TimePointParser, TimeRecurrenceParser +) from cylc.flow.cycling import parse_exclusion from cylc.flow.exceptions import ( - CylcTimeSyntaxError, CylcMissingContextPointError, - CylcMissingFinalCyclePointError) + CylcMissingContextPointError, + CylcMissingFinalCyclePointError, + CylcTimeSyntaxError, +) + +if TYPE_CHECKING: + from metomi.isodatetime.data import TimePoint + from cylc.flow.cycling.iso8601 import ISO8601Point UTC_UTC_OFFSET_HOURS_MINUTES = (0, 0) @@ -104,13 +113,19 @@ class CylcTimeParser: __slots__ = ('timepoint_parser', 'duration_parser', 'recurrence_parser', 'context_start_point', 'context_end_point') - def __init__(self, context_start_point, context_end_point, parsers): + def __init__( + self, + context_start_point: Union['ISO8601Point', str, None], + context_end_point: Union['ISO8601Point', str, None], + parsers: Tuple[TimePointParser, DurationParser, TimeRecurrenceParser] + ): if context_start_point is not None: context_start_point = str(context_start_point) if context_end_point is not None: context_end_point = str(context_end_point) self.timepoint_parser, self.duration_parser, self.recurrence_parser = ( - parsers) + parsers + ) if isinstance(context_start_point, str): context_start_point = self._get_point_from_expression( @@ -142,7 +157,7 @@ def initiate_parsers(num_expanded_year_digits=0, dump_format=None, else: dump_format = "CCYYMMDDThhmmZ" - timepoint_parser = metomi.isodatetime.parsers.TimePointParser( + timepoint_parser = TimePointParser( allow_only_basic=False, allow_truncated=True, num_expanded_year_digits=num_expanded_year_digits, @@ -150,10 +165,7 @@ def initiate_parsers(num_expanded_year_digits=0, dump_format=None, assumed_time_zone=assumed_time_zone ) - return (timepoint_parser, - metomi.isodatetime.parsers.DurationParser(), - metomi.isodatetime.parsers.TimeRecurrenceParser() - ) + return (timepoint_parser, DurationParser(), TimeRecurrenceParser()) def parse_interval(self, expr): """Parse an interval (duration) in full ISO date/time format.""" @@ -277,7 +289,7 @@ def parse_recurrence(self, expression, repetitions += 1 end_point = None - return metomi.isodatetime.data.TimeRecurrence( + return TimeRecurrence( repetitions=repetitions, start_point=start_point, duration=interval, @@ -310,13 +322,18 @@ def _get_interval_from_expression(self, expr, context=None): kwargs = {"minutes": 1} if not kwargs: return None - return metomi.isodatetime.data.Duration(**kwargs) + return Duration(**kwargs) return self.duration_parser.parse(expr) - def _get_min_from_expression(self, expr, context): - points = [x.strip() - for x in re.findall(self.MIN_REGEX, expr)[0].split(",")] - ptslist = [] + def _get_min_from_expression( + self, + expr: str, + context: 'TimePoint' + ) -> str: + points: List[str] = [ + x.strip() for x in re.findall(self.MIN_REGEX, expr)[0].split(",") + ] + ptslist: List['TimePoint'] = [] min_entry = "" for point in points: cpoint, offset = self._get_point_from_expression( @@ -331,8 +348,13 @@ def _get_min_from_expression(self, expr, context): min_entry = point return min_entry - def _get_point_from_expression(self, expr, context, is_required=False, - allow_truncated=False): + def _get_point_from_expression( + self, + expr: Optional[str], + context: 'TimePoint', + is_required: bool = False, + allow_truncated: bool = False + ) -> Tuple[Optional['TimePoint'], Optional[Duration]]: """Gets a TimePoint from an expression""" if expr is None: if is_required and allow_truncated: @@ -342,14 +364,14 @@ def _get_point_from_expression(self, expr, context, is_required=False, ) return context, None return None, None - expr_point = None - expr_offset = None + expr_point: Optional['TimePoint'] = None + expr_offset: Optional[Duration] = None if expr.startswith("min("): expr = self._get_min_from_expression(expr, context) if self.OFFSET_REGEX.search(expr): - chain_expr = self.CHAIN_REGEX.findall(expr) + chain_expr: List[str] = self.CHAIN_REGEX.findall(expr) expr = "" for item in chain_expr: if "P" not in item: @@ -368,7 +390,7 @@ def _get_point_from_expression(self, expr, context, is_required=False, return context, expr_offset for invalid_rec, msg in self.POINT_INVALID_FOR_CYLC_REGEXES: if invalid_rec.search(expr): - raise CylcTimeSyntaxError("'%s': %s" % (expr, msg)) + raise CylcTimeSyntaxError(f"'{expr}': {msg}") expr_to_parse = expr if expr.endswith("T"): expr_to_parse = expr + "00" @@ -389,5 +411,6 @@ def _get_point_from_expression(self, expr, context, is_required=False, continue return expr_point, expr_offset raise CylcTimeSyntaxError( - ("'%s': not a valid cylc-shorthand or full " % expr) + - "ISO 8601 date representation") + f"'{expr}': not a valid cylc-shorthand or full " + "ISO 8601 date representation" + ) From 0f093ab89c8c56a5e62b703b49aee1070013ccfa Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Fri, 10 Dec 2021 16:51:06 +0000 Subject: [PATCH 4/8] Un-unittest.TestCase-ify tests --- tests/unit/test_time_parser.py | 358 +++++++++++++++++---------------- 1 file changed, 184 insertions(+), 174 deletions(-) diff --git a/tests/unit/test_time_parser.py b/tests/unit/test_time_parser.py index 06f26cd618a..cfdb92d45ed 100644 --- a/tests/unit/test_time_parser.py +++ b/tests/unit/test_time_parser.py @@ -14,187 +14,197 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -import unittest +"""Test Cylc recurring date/time syntax parsing.""" -from cylc.flow.time_parser import CylcTimeParser, UTC_UTC_OFFSET_HOURS_MINUTES +import pytest +from cylc.flow.time_parser import ( + CylcTimeParser, + UTC_UTC_OFFSET_HOURS_MINUTES, +) -class TestRecurrenceWorkflow(unittest.TestCase): - """Test Cylc recurring date/time syntax parsing.""" - def setUp(self): - self._start_point = "19991226T0930Z" - # Note: the following timezone will be Z-ified *after* truncation - # or offsets are applied. - self._end_point = "20010506T1200+0200" - self._parsers = { - 0: CylcTimeParser( - self._start_point, self._end_point, - CylcTimeParser.initiate_parsers( - assumed_time_zone=UTC_UTC_OFFSET_HOURS_MINUTES - ) - ), - 2: CylcTimeParser( - self._start_point, self._end_point, - CylcTimeParser.initiate_parsers( - num_expanded_year_digits=2, - assumed_time_zone=UTC_UTC_OFFSET_HOURS_MINUTES - ) +@pytest.fixture +def parsers(): + _start_point = "19991226T0930Z" + # Note: the following timezone will be Z-ified *after* truncation + # or offsets are applied. + _end_point = "20010506T1200+0200" + return { + 0: CylcTimeParser( + _start_point, _end_point, + CylcTimeParser.initiate_parsers( + assumed_time_zone=UTC_UTC_OFFSET_HOURS_MINUTES ) - } + ), + 2: CylcTimeParser( + _start_point, _end_point, + CylcTimeParser.initiate_parsers( + num_expanded_year_digits=2, + assumed_time_zone=UTC_UTC_OFFSET_HOURS_MINUTES + ) + ) + } + + +@pytest.mark.parametrize( + 'expression, num_expanded_year_digits, ctrl_data, ctrl_results', + [ + ( + "R5/T06/04T1230-0300", + 0, + "R5/19991227T0600Z/20010604T1530Z", + ["19991227T0600Z", "20000506T1422Z", + "20000914T2245Z", "20010124T0707Z", + "20010604T1530Z"] + ), + ( + "R2/-0450000101T04Z/+0020630405T2200-05", + 2, + "R2/-0450000101T0400Z/+0020630406T0300Z", + ["-0450000101T0400Z", "+0020630406T0300Z"] + ), + ( + "R10/T12+P10D/-P1Y", + 0, + "R10/20000105T1200Z/20000506T1000Z", + ["20000105T1200Z", "20000119T0106Z", + "20000201T1413Z", "20000215T0320Z", + "20000228T1626Z", "20000313T0533Z", + "20000326T1840Z", "20000409T0746Z", + "20000422T2053Z", "20000506T1000Z"] + ) + ] +) +def test_first_recurrence_format( + expression, num_expanded_year_digits, ctrl_data, ctrl_results, + parsers +): + """Test the first ISO 8601 recurrence format.""" + parser = parsers[num_expanded_year_digits] + recurrence = parser.parse_recurrence(expression)[0] + test_data = str(recurrence) + assert test_data == ctrl_data + test_results = [str(res) for res in recurrence] + assert test_results == ctrl_results + + +def test_third_recurrence_format(parsers): + """Test the third ISO 8601 recurrence format.""" + tests = [("T06", "R/19991227T0600Z/P1D"), + ("T1230", "R/19991226T1230Z/P1D"), + ("04T00", "R/20000104T0000Z/P1M"), + ("01T06/PT2H", "R/20000101T0600Z/PT2H"), + ("0501T/P4Y3M", "R/20000501T0000Z/P4Y3M"), + ("P1YT5H", "R/19991226T0930Z/P1YT5H", + ["19991226T0930Z", "20001226T1430Z"]), + ("PT5M", "R/19991226T0930Z/PT5M"), + ("R/T-40", "R/19991226T0940Z/PT1H"), + ("R/150401T", "R/20150401T0000Z/P100Y"), + ("R5/T04-0300", "R5/19991227T0700Z/P1D"), + ("R2/T23Z/", "R2/19991226T2300Z/P1D"), + ("R/19991226T2359Z/P1W", "R/19991226T2359Z/P1W"), + ("R/-P1W/P1W", "R/19991219T0930Z/P1W"), + ("R/+P1D/P1Y", "R/19991227T0930Z/P1Y"), + ("R/T06-P1D/PT1H", "R/19991226T0600Z/PT1H"), + ("R/T12/PT10,5H", "R/19991226T1200Z/PT10,5H"), + ("R/T12/PT10.5H", "R/19991226T1200Z/PT10,5H"), + ("R5/+P10Y5M4D/PT2H", "R5/20100529T0930Z/PT2H"), + ("R30/-P200D/P10D", "R30/19990609T0930Z/P10D"), + ("R10/T06/PT2H", "R10/19991227T0600Z/PT2H", + ["19991227T0600Z", "19991227T0800Z", "19991227T1000Z", + "19991227T1200Z", "19991227T1400Z", "19991227T1600Z", + "19991227T1800Z", "19991227T2000Z", "19991227T2200Z", + "19991228T0000Z"]), + ("R//P1Y", "R/19991226T0930Z/P1Y", + ["19991226T0930Z", "20001226T0930Z"]), + ("R5//P1D", "R5/19991226T0930Z/P1D", + ["19991226T0930Z", "19991227T0930Z", + "19991228T0930Z", "19991229T0930Z", + "19991230T0930Z"]), + ("R1", "R1/19991226T0930Z/P0Y", + ["19991226T0930Z"])] + for test in tests: + if len(test) == 2: + expression, ctrl_data = test + ctrl_results = None + else: + expression, ctrl_data, ctrl_results = test + recurrence = (parsers[0].parse_recurrence(expression))[0] + test_data = str(recurrence) + assert test_data == ctrl_data + if ctrl_results is None: + continue + test_results = [] + for i, test_result in enumerate(recurrence): + if i > len(ctrl_results) - 1: + break + assert str(test_result) == ctrl_results[i] + test_results.append(str(test_result)) + assert test_results == ctrl_results + - def test_first_recurrence_format(self): - """Test the first ISO 8601 recurrence format.""" - tests = [("R5/T06/04T1230-0300", - 0, - "R5/19991227T0600Z/20010604T1530Z", - ["19991227T0600Z", "20000506T1422Z", - "20000914T2245Z", "20010124T0707Z", - "20010604T1530Z"]), - ("R2/-0450000101T04Z/+0020630405T2200-05", - 2, - "R2/-0450000101T0400Z/+0020630406T0300Z", - ["-0450000101T0400Z", "+0020630406T0300Z"]), - ("R10/T12+P10D/-P1Y", - 0, - "R10/20000105T1200Z/20000506T1000Z", - ["20000105T1200Z", "20000119T0106Z", - "20000201T1413Z", "20000215T0320Z", - "20000228T1626Z", "20000313T0533Z", - "20000326T1840Z", "20000409T0746Z", - "20000422T2053Z", "20000506T1000Z"])] - for test in tests: - if len(test) == 3: - expression, num_expanded_year_digits, ctrl_data = test - ctrl_results = None - else: - expression, num_expanded_year_digits, ctrl_data = test[:3] - ctrl_results = test[3] - parser = self._parsers[num_expanded_year_digits] - recurrence = parser.parse_recurrence(expression)[0] - test_data = str(recurrence) - self.assertEqual(test_data, ctrl_data) - if ctrl_results is None: - continue - test_results = [] - for i, test_result in enumerate(recurrence): - self.assertEqual(str(test_result), ctrl_results[i]) - test_results.append(str(test_result)) - self.assertEqual(test_results, ctrl_results) +def test_fourth_recurrence_format(parsers): + """Test the fourth ISO 8601 recurrence format.""" + tests = [("PT6H/20000101T0500Z", "R25/19991226T0500Z/PT6H"), + ("P12D/+P2W", "R44/19991221T1000Z/P12D"), + ("R2/P1W/-P1M1D", "R2/P1W/20010405T1000Z"), + ("R3/P6D/T12+02", "R3/P6D/20010506T1000Z"), + ("R4/P6DT12H/01T00+02", "R4/P6DT12H/20010531T2200Z"), + ("R5/P1D/20010506T1200+0200", "R5/P1D/20010506T1000Z"), + ("R6/PT5M/+PT2M", "R6/PT5M/20010506T1002Z"), + ("R7/P20Y/-P20Y", "R7/P20Y/19810506T1000Z"), + ("R8/P3YT2H/T18-02", "R8/P3YT2H/20010506T2000Z"), + ("R9/PT3H/31T", "R9/PT3H/20010531T0000Z"), + ("R10/P1Y/", "R10/P1Y/20010506T1000Z"), + ("R3/P2Y/02T", "R3/P2Y/20010602T0000Z"), + ("R/P2Y", "R2/19990506T1000Z/P2Y"), + ("R48/PT2H", "R48/PT2H/20010506T1000Z"), + ("R100/P21Y/", "R100/P21Y/20010506T1000Z")] + for test in tests: + if len(test) == 2: + expression, ctrl_data = test + ctrl_results = None + else: + expression, ctrl_data, ctrl_results = test + recurrence = (parsers[0].parse_recurrence( + expression))[0] + test_data = str(recurrence) + assert test_data == ctrl_data + if ctrl_results is None: + continue + test_results = [] + for i, test_result in enumerate(recurrence): + assert str(test_result) == ctrl_results[i] + test_results.append(str(test_result)) + assert test_results == ctrl_results - def test_third_recurrence_format(self): - """Test the third ISO 8601 recurrence format.""" - tests = [("T06", "R/19991227T0600Z/P1D"), - ("T1230", "R/19991226T1230Z/P1D"), - ("04T00", "R/20000104T0000Z/P1M"), - ("01T06/PT2H", "R/20000101T0600Z/PT2H"), - ("0501T/P4Y3M", "R/20000501T0000Z/P4Y3M"), - ("P1YT5H", "R/19991226T0930Z/P1YT5H", - ["19991226T0930Z", "20001226T1430Z"]), - ("PT5M", "R/19991226T0930Z/PT5M"), - ("R/T-40", "R/19991226T0940Z/PT1H"), - ("R/150401T", "R/20150401T0000Z/P100Y"), - ("R5/T04-0300", "R5/19991227T0700Z/P1D"), - ("R2/T23Z/", "R2/19991226T2300Z/P1D"), - ("R/19991226T2359Z/P1W", "R/19991226T2359Z/P1W"), - ("R/-P1W/P1W", "R/19991219T0930Z/P1W"), - ("R/+P1D/P1Y", "R/19991227T0930Z/P1Y"), - ("R/T06-P1D/PT1H", "R/19991226T0600Z/PT1H"), - ("R/T12/PT10,5H", "R/19991226T1200Z/PT10,5H"), - ("R/T12/PT10.5H", "R/19991226T1200Z/PT10,5H"), - ("R5/+P10Y5M4D/PT2H", "R5/20100529T0930Z/PT2H"), - ("R30/-P200D/P10D", "R30/19990609T0930Z/P10D"), - ("R10/T06/PT2H", "R10/19991227T0600Z/PT2H", - ["19991227T0600Z", "19991227T0800Z", "19991227T1000Z", - "19991227T1200Z", "19991227T1400Z", "19991227T1600Z", - "19991227T1800Z", "19991227T2000Z", "19991227T2200Z", - "19991228T0000Z"]), - ("R//P1Y", "R/19991226T0930Z/P1Y", - ["19991226T0930Z", "20001226T0930Z"]), - ("R5//P1D", "R5/19991226T0930Z/P1D", - ["19991226T0930Z", "19991227T0930Z", - "19991228T0930Z", "19991229T0930Z", - "19991230T0930Z"]), - ("R1", "R1/19991226T0930Z/P0Y", - ["19991226T0930Z"])] - for test in tests: - if len(test) == 2: - expression, ctrl_data = test - ctrl_results = None - else: - expression, ctrl_data, ctrl_results = test - recurrence = (self._parsers[0].parse_recurrence(expression))[0] - test_data = str(recurrence) - self.assertEqual(test_data, ctrl_data) - if ctrl_results is None: - continue - test_results = [] - for i, test_result in enumerate(recurrence): - if i > len(ctrl_results) - 1: - break - self.assertEqual(str(test_result), ctrl_results[i]) - test_results.append(str(test_result)) - self.assertEqual(test_results, ctrl_results) - def test_fourth_recurrence_format(self): - """Test the fourth ISO 8601 recurrence format.""" - tests = [("PT6H/20000101T0500Z", "R25/19991226T0500Z/PT6H"), - ("P12D/+P2W", "R44/19991221T1000Z/P12D"), - ("R2/P1W/-P1M1D", "R2/P1W/20010405T1000Z"), - ("R3/P6D/T12+02", "R3/P6D/20010506T1000Z"), - ("R4/P6DT12H/01T00+02", "R4/P6DT12H/20010531T2200Z"), - ("R5/P1D/20010506T1200+0200", "R5/P1D/20010506T1000Z"), - ("R6/PT5M/+PT2M", "R6/PT5M/20010506T1002Z"), - ("R7/P20Y/-P20Y", "R7/P20Y/19810506T1000Z"), - ("R8/P3YT2H/T18-02", "R8/P3YT2H/20010506T2000Z"), - ("R9/PT3H/31T", "R9/PT3H/20010531T0000Z"), - ("R10/P1Y/", "R10/P1Y/20010506T1000Z"), - ("R3/P2Y/02T", "R3/P2Y/20010602T0000Z"), - ("R/P2Y", "R2/19990506T1000Z/P2Y"), - ("R48/PT2H", "R48/PT2H/20010506T1000Z"), - ("R100/P21Y/", "R100/P21Y/20010506T1000Z")] - for test in tests: - if len(test) == 2: - expression, ctrl_data = test - ctrl_results = None - else: - expression, ctrl_data, ctrl_results = test - recurrence = (self._parsers[0].parse_recurrence( - expression))[0] - test_data = str(recurrence) - self.assertEqual(test_data, ctrl_data) - if ctrl_results is None: - continue - test_results = [] - for i, test_result in enumerate(recurrence): - self.assertEqual(str(test_result), ctrl_results[i]) - test_results.append(str(test_result)) - self.assertEqual(test_results, ctrl_results) +def test_inter_cycle_timepoints(parsers): + """Test the inter-cycle point parsing.""" + task_cycle_time = parsers[0].parse_timepoint("20000101T00Z") + tests = [("T06", "20000101T0600Z", 0), + ("-PT6H", "19991231T1800Z", 0), + ("+P5Y2M", "20050301T0000Z", 0), + ("0229T", "20000229T0000Z", 0), + ("+P54D", "20000224T0000Z", 0), + ("T12+P5W", "20000205T1200Z", 0), + ("-P1Y", "19990101T0000Z", 0), + ("-9999990101T00Z", "-9999990101T0000Z", 2), + ("20050601T2359+0200", "20050601T2159Z", 0)] + for expression, ctrl_data, num_expanded_year_digits in tests: + parser = parsers[num_expanded_year_digits] + test_data = str(parser.parse_timepoint( + expression, + context_point=task_cycle_time)) + assert test_data == ctrl_data - def test_inter_cycle_timepoints(self): - """Test the inter-cycle point parsing.""" - task_cycle_time = self._parsers[0].parse_timepoint("20000101T00Z") - tests = [("T06", "20000101T0600Z", 0), - ("-PT6H", "19991231T1800Z", 0), - ("+P5Y2M", "20050301T0000Z", 0), - ("0229T", "20000229T0000Z", 0), - ("+P54D", "20000224T0000Z", 0), - ("T12+P5W", "20000205T1200Z", 0), - ("-P1Y", "19990101T0000Z", 0), - ("-9999990101T00Z", "-9999990101T0000Z", 2), - ("20050601T2359+0200", "20050601T2159Z", 0)] - for expression, ctrl_data, num_expanded_year_digits in tests: - parser = self._parsers[num_expanded_year_digits] - test_data = str(parser.parse_timepoint( - expression, - context_point=task_cycle_time)) - self.assertEqual(test_data, ctrl_data) - def test_interval(self): - """Test the interval timepoint parsing (purposefully weak tests).""" - tests = ["PT6H2M", "PT6H", "P2Y5D", - "P5W", "PT12M2S", "PT65S", - "PT2M", "P1YT567,4M"] - for expression in tests: - test_data = str(self._parsers[0].parse_interval(expression)) - self.assertEqual(test_data, expression) +def test_interval(parsers): + """Test the interval timepoint parsing (purposefully weak tests).""" + tests = ["PT6H2M", "PT6H", "P2Y5D", + "P5W", "PT12M2S", "PT65S", + "PT2M", "P1YT567,4M"] + for expression in tests: + test_data = str(parsers[0].parse_interval(expression)) + assert test_data == expression From 9202fc5d13de48a72237f8fc2c730d908b5a5e05 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Fri, 10 Dec 2021 17:31:13 +0000 Subject: [PATCH 5/8] Fix datetime recurrence format 1 --- cylc/flow/time_parser.py | 2 +- .../functional/cyclers/51-recurrence_fmt_1.t | 22 +++++++++++++++++++ .../cyclers/51-recurrence_fmt_1/flow.cylc | 11 ++++++++++ .../cyclers/51-recurrence_fmt_1/reference.log | 6 +++++ tests/functional/lib/bash/test_header | 2 +- tests/unit/test_time_parser.py | 16 +++++++------- 6 files changed, 49 insertions(+), 10 deletions(-) create mode 100644 tests/functional/cyclers/51-recurrence_fmt_1.t create mode 100644 tests/functional/cyclers/51-recurrence_fmt_1/flow.cylc create mode 100644 tests/functional/cyclers/51-recurrence_fmt_1/reference.log diff --git a/cylc/flow/time_parser.py b/cylc/flow/time_parser.py index 8bc39f8c20f..8c16356954e 100644 --- a/cylc/flow/time_parser.py +++ b/cylc/flow/time_parser.py @@ -75,7 +75,7 @@ class CylcTimeParser: RECURRENCE_FORMAT_REGEXES = [ (re.compile(r"^(?P[^PR/][^/]*)$"), 3), - (re.compile(r"^R(?P\d+)/(?P[^PR/][^/]*)/(?P[^PR/]" + (re.compile(r"^R(?P\d+)?/(?P[^PR/][^/]*)/(?P[^PR/]" "[^/]*)$"), 1), (re.compile(r"^(?P[^PR/][^/]*)/(?PP[^/]*)/?$"), 3), (re.compile(r"^(?PP[^/]*)$"), 3), diff --git a/tests/functional/cyclers/51-recurrence_fmt_1.t b/tests/functional/cyclers/51-recurrence_fmt_1.t new file mode 100644 index 00000000000..6f90a79e4b5 --- /dev/null +++ b/tests/functional/cyclers/51-recurrence_fmt_1.t @@ -0,0 +1,22 @@ +#!/usr/bin/env bash +# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE. +# Copyright (C) NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +#------------------------------------------------------------------------------- +# Test ISO 8601 recurrence format no. 1 with unbounded repetitions +. "$(dirname "$0")/test_header" +set_test_number 2 +reftest +exit diff --git a/tests/functional/cyclers/51-recurrence_fmt_1/flow.cylc b/tests/functional/cyclers/51-recurrence_fmt_1/flow.cylc new file mode 100644 index 00000000000..8d83a47f1fa --- /dev/null +++ b/tests/functional/cyclers/51-recurrence_fmt_1/flow.cylc @@ -0,0 +1,11 @@ +[meta] + description = Test ISO 8601 recurrence format 1 +[scheduler] + UTC mode = True + allow implicit tasks = True + cycle point format = CCYY-MM-DD +[scheduling] + initial cycle point = 2010-01-01 + final cycle point = 2010-01-10 + [[graph]] + R/2010-01-01/2010-01-04 = worf # 3-day interval diff --git a/tests/functional/cyclers/51-recurrence_fmt_1/reference.log b/tests/functional/cyclers/51-recurrence_fmt_1/reference.log new file mode 100644 index 00000000000..dda1e323ebd --- /dev/null +++ b/tests/functional/cyclers/51-recurrence_fmt_1/reference.log @@ -0,0 +1,6 @@ +Initial point: 2010-01-01 +Final point: 2010-01-12 +2010-01-01/worf -triggered off [] +2010-01-04/worf -triggered off [] +2010-01-07/worf -triggered off [] +2010-01-10/worf -triggered off [] diff --git a/tests/functional/lib/bash/test_header b/tests/functional/lib/bash/test_header index 93f7d364519..f76d679ffec 100644 --- a/tests/functional/lib/bash/test_header +++ b/tests/functional/lib/bash/test_header @@ -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}}" install_workflow "$@" run_ok "${TEST_NAME}-validate" cylc validate "${WORKFLOW_NAME}" } diff --git a/tests/unit/test_time_parser.py b/tests/unit/test_time_parser.py index cfdb92d45ed..895a9cb1fba 100644 --- a/tests/unit/test_time_parser.py +++ b/tests/unit/test_time_parser.py @@ -54,9 +54,9 @@ def parsers(): "R5/T06/04T1230-0300", 0, "R5/19991227T0600Z/20010604T1530Z", - ["19991227T0600Z", "20000506T1422Z", - "20000914T2245Z", "20010124T0707Z", - "20010604T1530Z"] + ["19991227T0600Z", "20010604T1530Z", + "20021112T0100Z", "20040420T1030Z", + "20050927T2000Z"] ), ( "R2/-0450000101T04Z/+0020630405T2200-05", @@ -68,11 +68,11 @@ def parsers(): "R10/T12+P10D/-P1Y", 0, "R10/20000105T1200Z/20000506T1000Z", - ["20000105T1200Z", "20000119T0106Z", - "20000201T1413Z", "20000215T0320Z", - "20000228T1626Z", "20000313T0533Z", - "20000326T1840Z", "20000409T0746Z", - "20000422T2053Z", "20000506T1000Z"] + ["20000105T1200Z", "20000506T1000Z", + "20000905T0800Z", "20010105T0600Z", + "20010507T0400Z", "20010906T0200Z", + "20020106T0000Z", "20020507T2200Z", + "20020906T2000Z", "20030106T1800Z"] ) ] ) From 61cdf5daf47bab6ce4fe5c0d974241a14c4081a1 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Fri, 25 Feb 2022 19:02:29 +0000 Subject: [PATCH 6/8] Update changelog --- CHANGES.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 27a76e503bd..79fcbcdcdb8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -31,7 +31,16 @@ ones in. --> ------------------------------------------------------------------------------- ## __cylc-8.0rc3 (Pending)__ -### 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//`) +(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 From 85e0c083883632e32dbcdac1aa77cdb071c81599 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Mon, 4 Apr 2022 16:27:53 +0100 Subject: [PATCH 7/8] Warn instead of raise for zero-duration recurrence with >1 repetition --- cylc/flow/time_parser.py | 30 +++++++--- tests/functional/validate/51-zero-interval.t | 40 ------------- tests/unit/test_config.py | 63 +++++++++++++++++--- 3 files changed, 77 insertions(+), 56 deletions(-) delete mode 100755 tests/functional/validate/51-zero-interval.t diff --git a/cylc/flow/time_parser.py b/cylc/flow/time_parser.py index 8c16356954e..ac7f57af107 100644 --- a/cylc/flow/time_parser.py +++ b/cylc/flow/time_parser.py @@ -38,6 +38,7 @@ DurationParser, TimePointParser, TimeRecurrenceParser ) +from cylc.flow import LOG from cylc.flow.cycling import parse_exclusion from cylc.flow.exceptions import ( CylcMissingContextPointError, @@ -217,8 +218,8 @@ def parse_recurrence(self, expression, repetitions = int(repetitions) start = result.groupdict().get("start") end = result.groupdict().get("end") - start_required = (format_num in [1, 3]) - end_required = (format_num in [1, 4]) + start_required = (format_num in {1, 3}) + end_required = (format_num in {1, 4}) start_point, start_offset = self._get_point_from_expression( start, context_start_point, is_required=start_required, @@ -262,9 +263,8 @@ def parse_recurrence(self, expression, intv, context=intv_context_truncated_point) if format_num == 1: interval = None - if repetitions == 1: - # Set arbitrary interval (does not matter). - interval = self.duration_parser.parse("P0Y") + elif repetitions == 1: + interval = Duration(0) if start_point is not None: if start_point.truncated: start_point += context_start_point @@ -276,9 +276,11 @@ def parse_recurrence(self, expression, if end_offset is not None: end_point += end_offset - if (start_point is None and repetitions is None and - interval is not None and - context_start_point is not None): + if ( + interval and + start_point is None and repetitions is None and + context_start_point is not None + ): # isodatetime only reverses bounded end-point recurrences. # This is unbounded, and will come back in reverse order. # We need to reverse it. @@ -289,6 +291,14 @@ def parse_recurrence(self, expression, repetitions += 1 end_point = None + if not interval and repetitions != 1 and ( + (format_num != 1 or start_point == end_point) + ): + LOG.warning( + "Cannot have more than 1 repetition for zero-duration " + f"recurrence {expression}." + ) + return TimeRecurrence( repetitions=repetitions, start_point=start_point, @@ -298,7 +308,9 @@ def parse_recurrence(self, expression, raise CylcTimeSyntaxError("Could not parse %s" % expression) - def _get_interval_from_expression(self, expr, context=None): + def _get_interval_from_expression( + self, expr: Optional[str], context: Optional['TimePoint'] = None + ) -> Optional[Duration]: if expr is None: if context is None or not context.truncated: return None diff --git a/tests/functional/validate/51-zero-interval.t b/tests/functional/validate/51-zero-interval.t deleted file mode 100755 index dfa1058ff79..00000000000 --- a/tests/functional/validate/51-zero-interval.t +++ /dev/null @@ -1,40 +0,0 @@ -#!/usr/bin/env bash -# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE. -# Copyright (C) NIWA & British Crown (Met Office) & Contributors. -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . -#------------------------------------------------------------------------------- -# Test validation error message on zero-width date-time interval. -# See issue cylc/cylc-flow#1800. -. "$(dirname "$0")/test_header" -set_test_number 2 - -cat >'flow.cylc' <<'__FLOW_CONFIG__' -[scheduler] - UTC mode = True -[scheduling] - initial cycle point = 2010 - [[graph]] - P0M = foo # OOPS! zero-width interval -[runtime] - [[foo]] - script = true -__FLOW_CONFIG__ - -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. -__ERR__ - -exit diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 3c974f7db52..8c763ad1028 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -33,6 +33,8 @@ WorkflowConfigError, XtriggerConfigError, ) +from cylc.flow.scheduler_cli import RunOptions +from cylc.flow.scripts.validate import ValidateOptions from cylc.flow.workflow_files import WorkflowFiles from cylc.flow.wallclock import get_utc_mode, set_utc_mode from cylc.flow.xtrigger_mgr import XtriggerManager @@ -41,6 +43,7 @@ TASK_OUTPUT_SUCCEEDED ) + Fixture = Any @@ -1172,13 +1175,8 @@ def test_implicit_tasks( ) -@pytest.mark.parametrize( - 'workflow_meta,url_type', - product( - [True, False], - ['good', 'bad', 'ugly', 'broken'] - ) -) +@pytest.mark.parametrize('workflow_meta', [True, False]) +@pytest.mark.parametrize('url_type', ['good', 'bad', 'ugly', 'broken']) def test_process_urls(caplog, log_filter, workflow_meta, url_type): if url_type == 'good': @@ -1218,3 +1216,54 @@ def test_process_urls(caplog, log_filter, workflow_meta, url_type): caplog, contains='Detected deprecated template variables', ) + + +@pytest.mark.parametrize('opts', [ValidateOptions(), RunOptions()]) +@pytest.mark.parametrize( + 'recurrence, should_warn', + [ + # Format 3: + ('P0Y', True), + ('R//P0Y', True), + ('R2//P0Y', True), + ('R1//P0Y', False), + # Format 4: + ('R/P0M', True), + ('R1/P0M', False), + # Format 1: + ('R/2002-09-01/2002-09-01', True), + ('R1/2002-09-01/2002-09-01', False), + ('R/2002-08-31/2002-09-02', False), + ] +) +def test_zero_interval( + recurrence: str, + should_warn: bool, + opts: Values, + tmp_flow_config: Callable, + caplog: pytest.LogCaptureFixture, + log_filter: Callable, +): + """Test that a zero-duration recurrence with >1 repetition gets an + appropriate warning.""" + reg = 'ordinary' + flow_file: Path = tmp_flow_config(reg, f""" + [scheduler] + UTC mode = True + allow implicit tasks = True + [scheduling] + initial cycle point = 2002-08-30 + final cycle point = 2002-09-14 + [[graph]] + {recurrence} = slidescape36 + """) + WorkflowConfig(reg, flow_file, options=opts) + logged = log_filter( + caplog, + level=logging.WARNING, + contains="Cannot have more than 1 repetition for zero-duration" + ) + if should_warn: + assert logged + else: + assert not logged From bf76db8e190b6ec65bcb2bd24266bd57f5126790 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Tue, 5 Apr 2022 10:32:48 +0100 Subject: [PATCH 8/8] Tidy --- cylc/flow/cycling/iso8601.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/cylc/flow/cycling/iso8601.py b/cylc/flow/cycling/iso8601.py index 3a8872fb26d..d98f7067c66 100644 --- a/cylc/flow/cycling/iso8601.py +++ b/cylc/flow/cycling/iso8601.py @@ -696,7 +696,7 @@ def ingest_time(value: str, now: Optional['TimePoint'] = None) -> str: def prev_next( - value: str, now: 'TimePoint', parser: 'TimePointParser' + value: str, now: 'TimePoint', parser: 'TimePointParser' ) -> Tuple['TimePoint', Optional[str]]: """Handle prev() and next() syntax. @@ -720,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'] = [] @@ -763,15 +760,15 @@ 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 we don't call any of my_cp's methods - # (specifically, the ones that get cached) until after we've - # finished mutating it. I think. + # 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