Skip to content

Commit

Permalink
feat: deprecate dict methods
Browse files Browse the repository at this point in the history
towards #142
  • Loading branch information
tilsche committed Apr 14, 2023
1 parent 342375a commit 4a915a2
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 5 deletions.
11 changes: 10 additions & 1 deletion docs/upgrading.rst
Original file line number Diff line number Diff line change
Expand Up @@ -147,4 +147,13 @@ If you have imported types from it directly you should change to import directly
* :class:`TimeValue`
* :class:`TimeAggregate`
* :class:`Metric` (available in `metricq` since `5.0`)
* :class:`JsonDict` (available in `metricq` since `5.0`)
* :class:`JsonDict` (available in `metricq` since `5.0`)


Deprecation of `dict` methods
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The methods :meth:`TimeAggregate.dict` and :meth:`TimeValue.dict` have been deprecated.
Use the individual fields instead.
The using code has more context and should know better which fields to include.
In particular, whether to use :attr:`TimeAggregate.mean_sum` or :attr:`TimeAggregate.mean_integral` and which :attr:`TimeValue.timestamp` type to use.
38 changes: 36 additions & 2 deletions metricq/timeseries/time_aggregate.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from dataclasses import dataclass
from typing import Any
from typing import Any, Dict

from deprecated.sphinx import deprecated

from .. import history_pb2
from ..exceptions import NonMonotonicTimestamps
Expand Down Expand Up @@ -97,20 +99,52 @@ def integral_s(self) -> float:

@property
def mean(self) -> float:
"""
Mean value of this aggregate.
This is the general way to access the mean value if nothing specific
is known about the underlying raw data.
It may involve a heuristic to decide between :attr:`mean_integral` and
:attr:`mean_sum`. Currently, it defaults to :attr:`mean_integral`.
"""
if self.active_time.ns > 0:
return self.mean_integral
else:
return self.mean_sum

@property
def mean_integral(self) -> float:
"""
Mean value of this aggregate, calculated from the integral.
Use this if you want to explicitly force this property.
This should only be `NaN` if the aggregate interval is outside the
measured time.
"""
return self.integral_ns / self.active_time.ns

@property
def mean_sum(self) -> float:
"""
Mean value of this aggregate, calculated from the sum.
This can be useful when the underlying metric should be at regular
intervals, but there are gaps in the data. Otherwise,
:attr:`mean_integral` or just :attr:`mean` are more appropriate.
This value will be `NaN` if there are no raw data points in the
aggregate interval.
"""
return self.sum / self.count

def dict(self) -> dict[str, Any]:
@deprecated(
version="5.0.0",
reason=(
"Use the individual properties instead and chose between "
"`mean_integral` and `mean_sum` based on your use-case"
),
)
# using Dict as return type to work around https://github.com/python/mypy/issues/15047
def dict(self) -> Dict[str, Any]:
"""
returns a dict representing the TimeAggregate instance.
Keys are `timestamp`, `minimum`, `mean`, `maximum`, and `count`.
Expand Down
14 changes: 12 additions & 2 deletions metricq/timeseries/time_value.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from collections.abc import Iterator
from dataclasses import dataclass
from typing import Any
from typing import Any, Dict

from deprecated.sphinx import deprecated

from .timestamp import Timestamp

Expand All @@ -24,7 +26,15 @@ class TimeValue:
def __iter__(self) -> Iterator[Timestamp | float]:
return iter([self.timestamp, self.value])

def dict(self) -> dict[str, Any]:
@deprecated(
version="5.0.0",
reason=(
"Use the individual properties instead and select an appropriate "
"timestamp type."
),
)
# using Dict as return type to work around https://github.com/python/mypy/issues/15047
def dict(self) -> Dict[str, Any]:
"""
returns a dict representing the TimeValue instance.
Keys are `timestamp` and `value`
Expand Down
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ test =
typing =
mypy>=1.1.0
mypy-protobuf
types-Deprecated
types-setuptools
types-protobuf
types-python-dateutil
Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ def run(self):
# be available with the next aio-pika release
"aio-pika~=6.7, >=6.7.1",
get_protobuf_requirement(),
"Deprecated~=1.2.13",
"python-dateutil ~= 2.8, >=2.8.1",
"yarl",
"setuptools",
Expand Down

0 comments on commit 4a915a2

Please sign in to comment.