Skip to content
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
8 changes: 4 additions & 4 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -69,27 +69,27 @@ dev-mkdocs = [
"frequenz-repo-config[lib] == 0.18.0",
]
dev-mypy = [
"mypy == 2.1.0",
"mypy == 2.3.0",
"types-Markdown == 3.10.2.20260518",
"types-protobuf == 7.34.1.20260518",
"types-setuptools == 82.0.0.20260518",
# For checking the noxfile, docs/ script, and tests
"frequenz-sdk[dev-mkdocs,dev-noxfile,dev-pytest]",
]
dev-noxfile = ["nox == 2026.4.10", "frequenz-repo-config[lib] == 0.18.0"]
dev-noxfile = ["nox == 2026.7.11", "frequenz-repo-config[lib] == 0.18.0"]
dev-pylint = [
"pylint == 4.0.6",
# For checking the noxfile, docs/ script, and tests
"frequenz-sdk[dev-mkdocs,dev-noxfile,dev-pytest]",
]
dev-pytest = [
"pytest == 9.0.3",
"pytest == 9.1.1",
"frequenz-repo-config[extra-lint-examples] == 0.18.0",
"pytest-mock == 3.15.1",
"pytest-asyncio == 1.4.0",
"time-machine == 2.16.0",
"async-solipsism == 0.9",
"hypothesis == 6.155.7",
"hypothesis == 6.163.0",
]
dev = [
"frequenz-sdk[dev-mkdocs,dev-flake8,dev-formatting,dev-mkdocs,dev-mypy,dev-noxfile,dev-pylint,dev-pytest]",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

from frequenz.sdk.timeseries._resampling._wall_clock_timer import ClocksInfo

from .util import approx_time

_DEFAULT_MONOTONIC_REQUESTED_SLEEP = timedelta(seconds=1.0)
_DEFAULT_MONOTONIC_TIME = 1234.5
_DEFAULT_WALL_CLOCK_TIME = datetime(2023, 1, 1, tzinfo=timezone.utc)
Expand Down Expand Up @@ -141,7 +143,7 @@ def test_monotonic_drift(
monotonic_elapsed=monotonic_elapsed,
wall_clock_elapsed=_DEFAULT_WALL_CLOCK_ELAPSED,
)
assert info.monotonic_drift == pytest.approx(expected_drift)
assert info.monotonic_drift == approx_time(expected_drift)


@pytest.mark.parametrize(
Expand All @@ -166,7 +168,7 @@ def test_wall_clock_jump(
monotonic_elapsed=monotonic_elapsed,
wall_clock_elapsed=wall_clock_elapsed,
)
assert info.wall_clock_jump == pytest.approx(expected_jump)
assert info.wall_clock_jump == approx_time(expected_jump)


@dataclass(kw_only=True, frozen=True)
Expand Down Expand Up @@ -219,7 +221,7 @@ def test_wall_clock_factor(case: _TestCaseWallClockFactor) -> None:
wall_clock_elapsed=case.wall_clock_elapsed,
)
assert info.wall_clock_factor == pytest.approx(case.expected_factor)
assert info.wall_clock_to_monotonic(case.wall_clock_elapsed) == pytest.approx(
assert info.wall_clock_to_monotonic(case.wall_clock_elapsed) == approx_time(
case.monotonic_elapsed
)

Expand Down
10 changes: 6 additions & 4 deletions tests/timeseries/_resampling/wall_clock_timer/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,17 @@

from frequenz.sdk.timeseries._resampling._wall_clock_timer import WallClockTimerConfig

from .util import approx_time


def test_from_interval_defaults() -> None:
"""Test WallClockTimerConfig.from_interval() with only interval (all defaults)."""
interval = timedelta(seconds=10)
config = WallClockTimerConfig.from_interval(interval)
assert config.align_to == UNIX_EPOCH
assert config.async_drift_tolerance == pytest.approx(timedelta(seconds=1.0))
assert config.async_drift_tolerance == approx_time(timedelta(seconds=1.0))
assert config.wall_clock_drift_tolerance_factor == pytest.approx(0.1)
assert config.wall_clock_jump_threshold == pytest.approx(timedelta(seconds=10.0))
assert config.wall_clock_jump_threshold == approx_time(timedelta(seconds=10.0))


def test_from_interval_all_args() -> None:
Expand All @@ -38,9 +40,9 @@ def test_from_interval_all_args() -> None:
wall_clock_jump_threshold_factor=jump_factor,
)
assert config.align_to == align_to
assert config.async_drift_tolerance == pytest.approx(timedelta(seconds=1.0))
assert config.async_drift_tolerance == approx_time(timedelta(seconds=1.0))
assert config.wall_clock_drift_tolerance_factor == pytest.approx(0.3)
assert config.wall_clock_jump_threshold == pytest.approx(timedelta(seconds=2.0))
assert config.wall_clock_jump_threshold == approx_time(timedelta(seconds=2.0))


@pytest.mark.parametrize(
Expand Down
63 changes: 15 additions & 48 deletions tests/timeseries/_resampling/wall_clock_timer/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,10 @@
from collections.abc import Coroutine, Sequence
from dataclasses import dataclass, field
from datetime import datetime, timedelta, timezone
from typing import NamedTuple, TypeVar, assert_never, overload
from typing import Any, NamedTuple, TypeVar, assert_never, overload
from unittest.mock import MagicMock

import pytest

# This is not great, we are depending on an internal pytest API, but it is
# the most convenient way to provide a custom approx() comparison for datetime
# and timedelta.
# Other alternatives proven to be even more complex and hacky.
# It also looks like we are not the only ones doing this, see:
# https://github.com/pytest-dev/pytest/issues/8395
from _pytest.python_api import ApproxBase
from typing_extensions import override

from frequenz.sdk.timeseries import ClocksInfo, TickInfo
Expand Down Expand Up @@ -86,49 +78,24 @@ def mono_now() -> float:
return asyncio.get_running_loop().time()


# Pylint complains about abstract-method because _yield_comparisons is not implemented
# but it is used only in the default __eq__ method, which we are re-defining, so we can
# ignore it.
class approx_time(ApproxBase): # pylint: disable=invalid-name, abstract-method
def approx_time(
expected: datetime | timedelta,
*,
abs: timedelta = timedelta(milliseconds=1), # pylint: disable=redefined-builtin

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd rename it to something different rather than redefining the built-in abs and disabling the linter.
I'd suggest abs_tol as this is a common name used in math library.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure, this is a wrapper over pytest.approx() now, that uses abs as argument.

@daniel-zullo-frequenz daniel-zullo-frequenz Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I saw it before posting my comment and I think/thought what they are doing is not entirely correct (Even pytest documentation renders abs as built-in).
Probably short parameters like rel and abs make sense within the context and for testing purposes. But It is bad that is clashes with the built-in.
Internally they use long constant names:

    DEFAULT_ABSOLUTE_TOLERANCE = Decimal("1e-12")
    DEFAULT_RELATIVE_TOLERANCE = Decimal("1e-6")

Anyway it's is fine to keep it aligned with pytest but still doesn't seem totally right to me 馃懠

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Haha, I get your point, but in this case I think it makes sense to follow upstream. This is targeting users knowing the pytest.approx() API, we want the principle of the least surprise given the current context.

) -> Any:
"""Perform approximate comparisons for datetime or timedelta objects.

Inherits from `ApproxBase` to provide a rich comparison output in pytest.
"""
This is only a thin wrapper around `pytest.approx()` to default the tolerance to
1ms, as `pytest.approx()` requires an explicit tolerance for these types.

expected: datetime | timedelta
abs: timedelta

def __init__(
self,
expected: datetime | timedelta,
*,
abs: timedelta = timedelta(milliseconds=1), # pylint: disable=redefined-builtin
) -> None:
"""Initialize this instance."""
if abs < timedelta():
raise ValueError(
f"absolute tolerance must be a non-negative timedelta, not {abs}"
)
super().__init__(expected, abs=abs)
Args:
expected: The expected `datetime` or `timedelta` to compare against.
abs: The absolute tolerance as a `timedelta`. Defaults to 1ms.

def __repr__(self) -> str:
"""Return a string representation of this instance."""
return f"{self.expected} 卤 {self.abs}"

def __eq__(self, actual: object) -> bool:
"""Compare this instance with another object."""
# We need to split the cases for datetime and timedelta for type checking
# reasons.
diff: timedelta
match (self.expected, actual):
case (datetime(), datetime()):
diff = self.expected - actual
case (timedelta(), timedelta()):
diff = self.expected - actual
case _:
return NotImplemented

return abs(diff) <= self.abs
Returns:
An object comparing equal to any value within `abs` of `expected`.
"""
return pytest.approx(expected, abs=abs)
Comment thread
daniel-zullo-frequenz marked this conversation as resolved.


# We need to rewrite most of the attributes in these classes to use approximate
Expand Down