From 6dc264b168aa02af3f246d015ddf932687865862 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Mon, 28 Apr 2025 16:50:02 +0200 Subject: [PATCH 1/5] add hypothesis testing dependency --- environment.yml | 1 + pyproject.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/environment.yml b/environment.yml index 021a29d751..4b8694f0c4 100644 --- a/environment.yml +++ b/environment.yml @@ -28,6 +28,7 @@ dependencies: #! Keep in sync with [tool.pixi.dependencies] in pyproject.toml - pytest - pytest-html - coverage + - hypothesis # Typing - mypy diff --git a/pyproject.toml b/pyproject.toml index fddc2387db..0acf33ee75 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -79,6 +79,7 @@ trajan = "*" # Testing nbval = "*" pytest = "*" +hypothesis = "*" pytest-html = "*" coverage = "*" From 35d212bde7cfdc1bb583b06adab600fb870b3712 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Mon, 28 Apr 2025 18:00:07 +0200 Subject: [PATCH 2/5] Add TimeInterval helper and tests --- .gitignore | 1 + parcels/_core/utils/time.py | 50 ++++++++++++++++++++ tests/v4/test_gridadapter.py | 20 ++++---- tests/v4/utils/test_time.py | 91 ++++++++++++++++++++++++++++++++++++ 4 files changed, 152 insertions(+), 10 deletions(-) create mode 100644 parcels/_core/utils/time.py create mode 100644 tests/v4/utils/test_time.py diff --git a/.gitignore b/.gitignore index ac1b3b1980..50fb0120e0 100644 --- a/.gitignore +++ b/.gitignore @@ -27,6 +27,7 @@ parcels.egg-info/* dist/parcels*.egg parcels/_version_setup.py .pytest_cache +.hypothesis .coverage # pixi environments diff --git a/parcels/_core/utils/time.py b/parcels/_core/utils/time.py new file mode 100644 index 0000000000..64098c64ae --- /dev/null +++ b/parcels/_core/utils/time.py @@ -0,0 +1,50 @@ +from __future__ import annotations + +from datetime import datetime +from typing import Literal, TypeVar + +from cftime import datetime as cftime_datetime + +T = TypeVar("T", datetime, cftime_datetime) + + +class TimeInterval: + def __init__(self, left: T, right: T, closed: Literal["right", "left", "both", "neither"] = "left") -> None: + if not isinstance(left, (datetime, cftime_datetime)): + raise ValueError(f"Expected left to be a datetime or cftime_datetime, got {type(left)}.") + if not isinstance(right, (datetime, cftime_datetime)): + raise ValueError(f"Expected right to be a datetime or cftime_datetime, got {type(right)}.") + if left >= right: + raise ValueError(f"Expected left to be strictly less than right, got left={left} and right={right}.") + + if closed not in ["right", "left", "both", "neither"]: + raise ValueError(f"Invalid closed value: {closed}") + + self.left = left + self.right = right + self.closed = closed + + def __contains__(self, item: T) -> bool: + if self.closed == "left": + return self.left <= item < self.right + elif self.closed == "right": + return self.left < item <= self.right + elif self.closed == "both": + return self.left <= item <= self.right + elif self.closed == "neither": + return self.left < item < self.right + else: + raise ValueError(f"Invalid closed value: {self.closed}") + + def __repr__(self) -> str: + return f"TimeInterval(left={self.left!r}, right={self.right!r}, closed={self.closed!r})" + + def __eq__(self, other: object) -> bool: + if not isinstance(other, TimeInterval): + return False + return self.left == other.left and self.right == other.right and self.closed == other.closed + + def __ne__(self, other: object) -> bool: + return not self.__eq__(other) + + def intersection(self, other: TimeInterval) -> TimeInterval: ... diff --git a/tests/v4/test_gridadapter.py b/tests/v4/test_gridadapter.py index 24c67b70b6..9f1ead0ac7 100644 --- a/tests/v4/test_gridadapter.py +++ b/tests/v4/test_gridadapter.py @@ -10,18 +10,18 @@ from parcels.v4.grid import Grid as NewGrid from parcels.v4.gridadapter import GridAdapter -TestCase = namedtuple("TestCase", ["Grid", "attr", "expected"]) +GridTestCase = namedtuple("GridTestCase", ["Grid", "attr", "expected"]) test_cases = [ - TestCase(datasets["ds_2d_left"], "lon", datasets["ds_2d_left"].XG.values), - TestCase(datasets["ds_2d_left"], "lat", datasets["ds_2d_left"].YG.values), - TestCase(datasets["ds_2d_left"], "depth", datasets["ds_2d_left"].ZG.values), - TestCase(datasets["ds_2d_left"], "time", datasets["ds_2d_left"].time.values), - TestCase(datasets["ds_2d_left"], "xdim", N), - TestCase(datasets["ds_2d_left"], "ydim", 2 * N), - TestCase(datasets["ds_2d_left"], "zdim", 3 * N), - TestCase(datasets["ds_2d_left"], "tdim", T), - TestCase(datasets["ds_2d_left"], "time_origin", TimeConverter(datasets["ds_2d_left"].time.values[0])), + GridTestCase(datasets["ds_2d_left"], "lon", datasets["ds_2d_left"].XG.values), + GridTestCase(datasets["ds_2d_left"], "lat", datasets["ds_2d_left"].YG.values), + GridTestCase(datasets["ds_2d_left"], "depth", datasets["ds_2d_left"].ZG.values), + GridTestCase(datasets["ds_2d_left"], "time", datasets["ds_2d_left"].time.values), + GridTestCase(datasets["ds_2d_left"], "xdim", N), + GridTestCase(datasets["ds_2d_left"], "ydim", 2 * N), + GridTestCase(datasets["ds_2d_left"], "zdim", 3 * N), + GridTestCase(datasets["ds_2d_left"], "tdim", T), + GridTestCase(datasets["ds_2d_left"], "time_origin", TimeConverter(datasets["ds_2d_left"].time.values[0])), ] diff --git a/tests/v4/utils/test_time.py b/tests/v4/utils/test_time.py new file mode 100644 index 0000000000..ccef154f25 --- /dev/null +++ b/tests/v4/utils/test_time.py @@ -0,0 +1,91 @@ +from __future__ import annotations + +from datetime import datetime, timedelta + +import pytest +from cftime import datetime as cftime_datetime +from hypothesis import given +from hypothesis import strategies as st + +from parcels._core.utils.time import TimeInterval + +calendar_strategy = st.sampled_from(["gregorian", "proleptic_gregorian", "365_day", "360_day", "julian", "366_day"]) +closed_strategy = st.sampled_from(["right", "left", "both", "neither"]) + + +@st.composite +def cftime_datetime_strategy(draw, calendar=None): + year = draw(st.integers(1900, 2100)) + month = draw(st.integers(1, 12)) + day = draw(st.integers(1, 28)) + if calendar is None: + calendar = draw(calendar_strategy) + return cftime_datetime(year, month, day, calendar=calendar) + + +@st.composite +def cftime_interval_strategy(draw, left=None): + if left is None: + left = draw(cftime_datetime_strategy()) + right = left + draw( + st.timedeltas( + min_value=timedelta(seconds=1), + max_value=timedelta(days=100 * 365), + ) + ) + closed = draw(closed_strategy) + return TimeInterval(left, right, closed) + + +@pytest.mark.parametrize("closed", ["right", "left", "both", "neither"]) +@pytest.mark.parametrize( + "left,right", + [ + (cftime_datetime(2023, 1, 1, calendar="gregorian"), cftime_datetime(2023, 1, 2, calendar="gregorian")), + (cftime_datetime(2023, 6, 1, calendar="365_day"), cftime_datetime(2023, 6, 2, calendar="365_day")), + (cftime_datetime(2023, 12, 1, calendar="360_day"), cftime_datetime(2023, 12, 2, calendar="360_day")), + ], +) +def test_time_interval_initialization(left, right, closed): + """Test that TimeInterval can be initialized with valid inputs.""" + interval = TimeInterval(left, right, closed) + assert interval.left == left + assert interval.right == right + assert interval.closed == closed + + with pytest.raises(ValueError): + TimeInterval(right, left, closed) + + +def test_time_interval_invalid_closed(): + """Test that TimeInterval raises ValueError for invalid closed values.""" + left = datetime(2023, 1, 1) + right = datetime(2023, 1, 2) + with pytest.raises(ValueError): + TimeInterval(left, right, closed="invalid") + + +@given(cftime_interval_strategy()) +def test_time_interval_contains(interval): + left = interval.left + right = interval.right + middle = left + (right - left) / 2 + + if interval.closed in ["left", "both"]: + assert left in interval + if interval.closed in ["right", "both"]: + assert right in interval + + assert middle in interval + + +def test_time_interval_repr(): + """Test the string representation of TimeInterval.""" + interval = TimeInterval(datetime(2023, 1, 1, 12, 0), datetime(2023, 1, 2, 12, 0), "both") + expected = "TimeInterval(left=datetime.datetime(2023, 1, 1, 12, 0), right=datetime.datetime(2023, 1, 2, 12, 0), closed='both')" + assert repr(interval) == expected + + +@given(cftime_interval_strategy()) +def test_time_interval_equality(interval): + assert interval == interval From dffee4b559de54985e6cfdecb390be3d177fcdf1 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Tue, 29 Apr 2025 13:29:14 +0200 Subject: [PATCH 3/5] Update time helper with .intersection() and remove closedness --- parcels/_core/utils/time.py | 71 ++++++++++++++++--------- tests/v4/utils/test_time.py | 103 ++++++++++++++++++++++++++++-------- 2 files changed, 126 insertions(+), 48 deletions(-) diff --git a/parcels/_core/utils/time.py b/parcels/_core/utils/time.py index 64098c64ae..8b658faeb1 100644 --- a/parcels/_core/utils/time.py +++ b/parcels/_core/utils/time.py @@ -1,50 +1,71 @@ from __future__ import annotations from datetime import datetime -from typing import Literal, TypeVar +from typing import TypeVar -from cftime import datetime as cftime_datetime +import cftime -T = TypeVar("T", datetime, cftime_datetime) +T = TypeVar("T", datetime, cftime.datetime) class TimeInterval: - def __init__(self, left: T, right: T, closed: Literal["right", "left", "both", "neither"] = "left") -> None: - if not isinstance(left, (datetime, cftime_datetime)): - raise ValueError(f"Expected left to be a datetime or cftime_datetime, got {type(left)}.") - if not isinstance(right, (datetime, cftime_datetime)): - raise ValueError(f"Expected right to be a datetime or cftime_datetime, got {type(right)}.") + """A class representing a time interval between two datetime objects. + + Parameters + ---------- + left : datetime or cftime.datetime + The left endpoint of the interval. + right : datetime or cftime.datetime + The right endpoint of the interval. + + Notes + ----- + For the purposes of this codebase, the interval can be thought of as closed on the left and right. + """ + + def __init__(self, left: T, right: T) -> None: + if not isinstance(left, (datetime, cftime.datetime)): + raise ValueError(f"Expected left to be a datetime or cftime.datetime, got {type(left)}.") + if not isinstance(right, (datetime, cftime.datetime)): + raise ValueError(f"Expected right to be a datetime or cftime.datetime, got {type(right)}.") if left >= right: raise ValueError(f"Expected left to be strictly less than right, got left={left} and right={right}.") - - if closed not in ["right", "left", "both", "neither"]: - raise ValueError(f"Invalid closed value: {closed}") + if not _is_compatible(left, right): + raise ValueError(f"Expected left and right to be compatible, got left={left} and right={right}.") self.left = left self.right = right - self.closed = closed def __contains__(self, item: T) -> bool: - if self.closed == "left": - return self.left <= item < self.right - elif self.closed == "right": - return self.left < item <= self.right - elif self.closed == "both": - return self.left <= item <= self.right - elif self.closed == "neither": - return self.left < item < self.right - else: - raise ValueError(f"Invalid closed value: {self.closed}") + return self.left <= item <= self.right def __repr__(self) -> str: - return f"TimeInterval(left={self.left!r}, right={self.right!r}, closed={self.closed!r})" + return f"TimeInterval(left={self.left!r}, right={self.right!r})" def __eq__(self, other: object) -> bool: if not isinstance(other, TimeInterval): return False - return self.left == other.left and self.right == other.right and self.closed == other.closed + return self.left == other.left and self.right == other.right def __ne__(self, other: object) -> bool: return not self.__eq__(other) - def intersection(self, other: TimeInterval) -> TimeInterval: ... + def intersection(self, other: TimeInterval) -> TimeInterval | None: + """Return the intersection of two time intervals. Returns None if there is no overlap.""" + try: + start = max(self.left, other.left) + end = min(self.right, other.right) + except Exception as e: + raise ValueError("TimeIntervals are not compatible.") from e + + return TimeInterval(start, end) if start <= end else None + + +def _is_compatible(t1: datetime | cftime.datetime, t2: datetime | cftime.datetime) -> bool: + """Checks whether two (cftime.)datetime objects are compatible.""" + try: + t1 - t2 + except Exception: + return False + else: + return True diff --git a/tests/v4/utils/test_time.py b/tests/v4/utils/test_time.py index ccef154f25..ad09b9787e 100644 --- a/tests/v4/utils/test_time.py +++ b/tests/v4/utils/test_time.py @@ -10,7 +10,6 @@ from parcels._core.utils.time import TimeInterval calendar_strategy = st.sampled_from(["gregorian", "proleptic_gregorian", "365_day", "360_day", "julian", "366_day"]) -closed_strategy = st.sampled_from(["right", "left", "both", "neither"]) @st.composite @@ -33,11 +32,9 @@ def cftime_interval_strategy(draw, left=None): max_value=timedelta(days=100 * 365), ) ) - closed = draw(closed_strategy) - return TimeInterval(left, right, closed) + return TimeInterval(left, right) -@pytest.mark.parametrize("closed", ["right", "left", "both", "neither"]) @pytest.mark.parametrize( "left,right", [ @@ -46,23 +43,14 @@ def cftime_interval_strategy(draw, left=None): (cftime_datetime(2023, 12, 1, calendar="360_day"), cftime_datetime(2023, 12, 2, calendar="360_day")), ], ) -def test_time_interval_initialization(left, right, closed): +def test_time_interval_initialization(left, right): """Test that TimeInterval can be initialized with valid inputs.""" - interval = TimeInterval(left, right, closed) + interval = TimeInterval(left, right) assert interval.left == left assert interval.right == right - assert interval.closed == closed with pytest.raises(ValueError): - TimeInterval(right, left, closed) - - -def test_time_interval_invalid_closed(): - """Test that TimeInterval raises ValueError for invalid closed values.""" - left = datetime(2023, 1, 1) - right = datetime(2023, 1, 2) - with pytest.raises(ValueError): - TimeInterval(left, right, closed="invalid") + TimeInterval(right, left) @given(cftime_interval_strategy()) @@ -71,21 +59,90 @@ def test_time_interval_contains(interval): right = interval.right middle = left + (right - left) / 2 - if interval.closed in ["left", "both"]: - assert left in interval - if interval.closed in ["right", "both"]: - assert right in interval - + assert left in interval + assert right in interval assert middle in interval def test_time_interval_repr(): """Test the string representation of TimeInterval.""" - interval = TimeInterval(datetime(2023, 1, 1, 12, 0), datetime(2023, 1, 2, 12, 0), "both") - expected = "TimeInterval(left=datetime.datetime(2023, 1, 1, 12, 0), right=datetime.datetime(2023, 1, 2, 12, 0), closed='both')" + interval = TimeInterval(datetime(2023, 1, 1, 12, 0), datetime(2023, 1, 2, 12, 0)) + expected = "TimeInterval(left=datetime.datetime(2023, 1, 1, 12, 0), right=datetime.datetime(2023, 1, 2, 12, 0))" assert repr(interval) == expected @given(cftime_interval_strategy()) def test_time_interval_equality(interval): assert interval == interval + + +@pytest.mark.parametrize( + "interval1,interval2,expected", + [ + pytest.param( + TimeInterval( + cftime_datetime(2023, 1, 1, calendar="gregorian"), cftime_datetime(2023, 1, 3, calendar="gregorian") + ), + TimeInterval( + cftime_datetime(2023, 1, 2, calendar="gregorian"), cftime_datetime(2023, 1, 4, calendar="gregorian") + ), + TimeInterval( + cftime_datetime(2023, 1, 2, calendar="gregorian"), cftime_datetime(2023, 1, 3, calendar="gregorian") + ), + id="overlapping intervals", + ), + pytest.param( + TimeInterval( + cftime_datetime(2023, 1, 1, calendar="gregorian"), cftime_datetime(2023, 1, 3, calendar="gregorian") + ), + TimeInterval( + cftime_datetime(2023, 1, 5, calendar="gregorian"), cftime_datetime(2023, 1, 6, calendar="gregorian") + ), + None, + id="non-overlapping intervals", + ), + pytest.param( + TimeInterval( + cftime_datetime(2023, 1, 1, calendar="gregorian"), cftime_datetime(2023, 1, 3, calendar="gregorian") + ), + TimeInterval( + cftime_datetime(2023, 1, 1, calendar="gregorian"), cftime_datetime(2023, 1, 2, calendar="gregorian") + ), + TimeInterval( + cftime_datetime(2023, 1, 1, calendar="gregorian"), cftime_datetime(2023, 1, 2, calendar="gregorian") + ), + id="intervals with same start time", + ), + pytest.param( + TimeInterval( + cftime_datetime(2023, 1, 1, calendar="gregorian"), cftime_datetime(2023, 1, 3, calendar="gregorian") + ), + TimeInterval( + cftime_datetime(2023, 1, 2, calendar="gregorian"), cftime_datetime(2023, 1, 3, calendar="gregorian") + ), + TimeInterval( + cftime_datetime(2023, 1, 2, calendar="gregorian"), cftime_datetime(2023, 1, 3, calendar="gregorian") + ), + id="intervals with same end time", + ), + ], +) +def test_time_interval_intersection(interval1, interval2, expected): + """Test the intersection of two time intervals.""" + result = interval1.intersection(interval2) + if expected is None: + assert result is None + else: + assert result.left == expected.left + assert result.right == expected.right + + +def test_time_interval_intersection_different_calendars(): + interval1 = TimeInterval( + cftime_datetime(2023, 1, 1, calendar="gregorian"), cftime_datetime(2023, 1, 3, calendar="gregorian") + ) + interval2 = TimeInterval( + cftime_datetime(2023, 1, 1, calendar="365_day"), cftime_datetime(2023, 1, 3, calendar="365_day") + ) + with pytest.raises(ValueError, match="TimeIntervals are not compatible."): + interval1.intersection(interval2) From a9552bb73a002a5180e4d50df1e8b187aa898483 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Tue, 29 Apr 2025 13:44:16 +0200 Subject: [PATCH 4/5] Add TimeInterval.intersection properties --- tests/v4/utils/test_time.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/v4/utils/test_time.py b/tests/v4/utils/test_time.py index ad09b9787e..c572df2eef 100644 --- a/tests/v4/utils/test_time.py +++ b/tests/v4/utils/test_time.py @@ -23,9 +23,9 @@ def cftime_datetime_strategy(draw, calendar=None): @st.composite -def cftime_interval_strategy(draw, left=None): +def cftime_interval_strategy(draw, left=None, calendar=None): if left is None: - left = draw(cftime_datetime_strategy()) + left = draw(cftime_datetime_strategy(calendar=calendar)) right = left + draw( st.timedeltas( min_value=timedelta(seconds=1), @@ -64,6 +64,16 @@ def test_time_interval_contains(interval): assert middle in interval +@given(cftime_interval_strategy(calendar="365_day"), cftime_interval_strategy(calendar="365_day")) +def test_time_interval_intersection_commutative(interval1, interval2): + assert interval1.intersection(interval2) == interval2.intersection(interval1) + + +@given(cftime_interval_strategy()) +def test_time_interval_intersection_with_self(interval): + assert interval.intersection(interval) == interval + + def test_time_interval_repr(): """Test the string representation of TimeInterval.""" interval = TimeInterval(datetime(2023, 1, 1, 12, 0), datetime(2023, 1, 2, 12, 0)) From 5905416654a28c343e85a624440a74d3dcec4d39 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Tue, 29 Apr 2025 13:51:59 +0200 Subject: [PATCH 5/5] make is_compatible public for developers --- parcels/_core/utils/time.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/parcels/_core/utils/time.py b/parcels/_core/utils/time.py index 8b658faeb1..917be1a4d9 100644 --- a/parcels/_core/utils/time.py +++ b/parcels/_core/utils/time.py @@ -30,7 +30,7 @@ def __init__(self, left: T, right: T) -> None: raise ValueError(f"Expected right to be a datetime or cftime.datetime, got {type(right)}.") if left >= right: raise ValueError(f"Expected left to be strictly less than right, got left={left} and right={right}.") - if not _is_compatible(left, right): + if not is_compatible(left, right): raise ValueError(f"Expected left and right to be compatible, got left={left} and right={right}.") self.left = left @@ -52,16 +52,16 @@ def __ne__(self, other: object) -> bool: def intersection(self, other: TimeInterval) -> TimeInterval | None: """Return the intersection of two time intervals. Returns None if there is no overlap.""" - try: - start = max(self.left, other.left) - end = min(self.right, other.right) - except Exception as e: - raise ValueError("TimeIntervals are not compatible.") from e + if not is_compatible(self.left, other.left): + raise ValueError("TimeIntervals are not compatible.") + + start = max(self.left, other.left) + end = min(self.right, other.right) return TimeInterval(start, end) if start <= end else None -def _is_compatible(t1: datetime | cftime.datetime, t2: datetime | cftime.datetime) -> bool: +def is_compatible(t1: datetime | cftime.datetime, t2: datetime | cftime.datetime) -> bool: """Checks whether two (cftime.)datetime objects are compatible.""" try: t1 - t2