Skip to content

match … case float(): is unsafe on any float-typed field #250

Description

@llucax

What happened?

Whenever this library annotates a dataclass field as float | Other (or plain float) and later unwraps it with match … case float():, the code has a latent AssertionError on integer inputs — despite the calling code passing mypy --strict.

See for example how it is handled in :

@dataclass(frozen=True, kw_only=True)
class MetricSample:
    """A sampled metric.
    ...
    """
    ...
    value: float | AggregatedMetricValue | None
    """The value of the sampled metric."""

    def as_single_value(
        self, *, aggregation_method: AggregationMethod = AggregationMethod.AVG
    ) -> float | None:
        """Return the value of this sample as a single value.
        ...
        """
        match self.value:
            case float() | int():  # <---------------- Without `int()` we are screwed
                return self.value
            case AggregatedMetricValue():
                ...
            case None:
                return None
            case unexpected:
                assert_never(unexpected)

This is a fundamental mismatch between Python's static and runtime type systems, not a bug we can hack around cleanly. Every mitigation we have identified has meaningful drawbacks. Even a bool can be passed where a float is expected.

We could be careful and do it right in out code but this keeps being a hazard to our users if they want to access .value and pattern match directly with it.

What did you expect instead?

Users using:

        metric_sample = MetricSample(value=1, ...)
        match metric_sample.value:
            case float():
                return self.value
            case AggregatedMetricValue():
                ...
            case None:
                return None
            case unexpected:
                assert_never(unexpected)

Don't get an unexpected assertion because the actual value type is int, not float.

Affected version(s)

No response

Affected part(s)

I don't know (part:❓)

Extra information

We prototyped the most obvious fix (runtime coercion at ingress — Option 1 below).

It works, but the measured performance impact on hot-path types made us stop and file this issue instead of landing it. This ticket documents the trap, lists the options (including "just live with it"), and asks how we want to handle it in this library.

The bug

Reduced to a self-contained example:

from dataclasses import dataclass
from typing import assert_never

@dataclass(frozen=True)
class Wrapper:
    value: float | str

    def unwrap(self) -> float:
        match self.value:
            case float() as f:
                return f
            case str() as s:
                return float(s)
            case unknown:
                assert_never(unknown)  # nominally unreachable

>>> Wrapper(value=1).unwrap()          # int is assignable to float under PEP 484
Traceback (most recent call last):
  ...
AssertionError                         # but isinstance(1, float) is False

The stored 1 is an int; case float(): is an isinstance(_, float) check which is False; the value falls through the exhaustive-looking match to assert_never.

Any field annotated as float | Other (including float | None, float | SomeClass, etc.) that is later unwrapped by match has this shape. Widening the arm to case float() | int(): fixes int/bool but moves the same crash to Decimal, Fraction, and NumPy non-float scalars.

Why Python offers no clean solution

The numeric tower cannot be turned off

x: float = 1 type-checks under mypy, pyright, basedpyright, and every mainstream Python type-checker. mypy --strict does not disable this — PEP 484's numeric tower is baked into the type system. Even inside a library where every caller uses --strict, all of the following are legal and produce integer runtime values in float-annotated storage:

def load(v: int) -> None:
    x: float = v              # legal
    Bounds(lower=x)           # legal; stores int at runtime

raw: Sequence[float] = [1, 2, 3]              # legal; list[int] is a Sequence[float]
AggregatedMetricValue(avg=1.0, raw=raw)       # legal; raw contains ints at runtime

dataclasses.replace(bounds_instance, lower=1) # legal; dispatches through the field annotation

json.loads('{"lower": 0}')                    # produces int; caller may forward it as float
The typing ABCs are not recognized
  • numbers.Real / numbers.Rational / numbers.Integral: typeshed's own numbers.pyi warns "float is not seen as a subtype of Real"; no mainstream type-checker implements the numeric-ABC hierarchy. Decimal is not even in the tower.
  • typing.SupportsFloat is too permissive — anything with __float__ matches, including str (via float("1.5")).
match … case T(): is isinstance-based

There is no way to write a case arm that matches "float including its numeric-tower siblings" without spelling every accepted runtime type explicitly, which just re-creates the same problem for the next unknown type that appears.

Structural typing hacks don't fully close it either (see Option 3 below)

A Protocol whose methods exist on float but not on int/bool can carve out direct int literals at construction time, but the numeric tower still lets int slip in through widened float variables, Sequence[float] covariance, dataclasses.replace(), and other paths.

Where this trap lives in this library

Every dataclass with a float-typed field is subject to the trap. Only accessors that currently use case float(): (or its widened case float() | int(): sibling) crash today; fields without accessors are traps waiting for a future accessor addition.

Released today (v0.4.0):

Class Field(s) Accessor with case? Status
MetricSample value: float | AggregatedMetricValue | None as_single_value() uses widened case float() | int(): Latent crash on Decimal / Fraction / other reals
AggregatedMetricValue avg: float, min/max: float | None, raw: Sequence[float] none Trap if an accessor is ever added
Bounds lower/upper: float | None none Trap if an accessor is ever added

In-progress feature branches also add:

Class Field(s) Accessor with case?
Location latitude/longitude: float | Invalid* get_latitude() / get_longitude() use case float():crashes today on int inputs
InvalidLatitude, InvalidLongitude value: float none
PowerTransformer primary/secondary_voltage: float none

MetricSample, AggregatedMetricValue, and Bounds are constructed many times per second per component in typical Frequenz deployments — any per-construction cost on those types is measurable in production.

Options

Option 1 — Coerce at ingress (runtime cost) — prototyped

We prototyped this approach on a working branch across every class listed above. It adds __post_init__ to each frozen dataclass and normalizes every float-typed field to a real float via object.__setattr__:

def __post_init__(self) -> None:
    object.__setattr__(self, "avg", float(self.avg))
    if self.min is not None:
        object.__setattr__(self, "min", float(self.min))
    if self.max is not None:
        object.__setattr__(self, "max", float(self.max))
    object.__setattr__(self, "raw", [float(x) for x in self.raw])

The prototype passes nox (pytest with warnings-as-errors, mypy --strict, pylint 10.00/10). Functionally the approach is correct. What stopped us from landing it was the measured performance impact on hot-path types, which is the reason for filing this issue rather than merging the prototype.

Pros

  • Fully correct at runtime for every ingress path (typed callers, untyped callers, dataclasses.replace(), copy, pickle, JSON/dict inflation, protobuf converters).
  • case float(): becomes reliable; assert_never fall-throughs become genuinely unreachable.
  • Sequence[float] element coercion incidentally builds a defensive copy — protects against caller mutation of raw after construction.

Cons — the reason this issue exists

  • Measured on Python 3.14 with the prototype, AggregatedMetricValue(avg=1, min=1, max=1, raw=[1..9]) is ~2.3× slower after coercion:
    # before the prototype
    500000 loops, best of 5:  895 nsec per loop
    # after the prototype
    100000 loops, best of 5: 2.06 usec per loop
    
  • Cost is unconditional — float(x) is called even when x is already float (the common case in protobuf-driven construction).
  • On the hot path (MetricSample, AggregatedMetricValue, Bounds — constructed hundreds of times per second per component), this compounds across the fleet.
Option 2 — Widen match arms to case float() | int():

Narrowest possible change:

match self.value:
    case float() | int() as v:
        return float(v)
    ...

Pros

  • Zero construction overhead.
  • Fixes the immediate int (and bool) crash.

Cons

  • Doesn't fix the underlying problem for Decimal, Fraction, numpy.float32 / float16 / longdouble (which are numpy.floating scalars, not built-in float subclasses).
  • Stores heterogeneous runtime types under an annotation that claims float; every downstream user of the field has to re-coerce or handle the ambiguity.
  • Silently accepts bool (since bool ⊂ int), which may or may not be desired.
Option 3 — Public structural Protocol for constructor parameters

Define a public Float protocol whose methods are on float but not on int/bool, and use it as the constructor parameter type (storage annotation stays float):

from typing import Protocol

class Float(Protocol):
    """Structural stand-in for `float` that excludes `int` and `bool`.

    Python's numeric tower makes `int` and `bool` assignable to `float`
    under PEP 484, which breaks `match … case float():` at runtime.
    This protocol carves out just the exact-`float` values by matching
    methods present on `float` but not on `int` / `bool`.
    """
    def hex(self) -> str: ...
    def as_integer_ratio(self) -> tuple[int, int]: ...
    def is_integer(self) -> bool: ...


@dataclass(frozen=True, slots=True, init=False)
class Bounds:
    lower: float | None
    upper: float | None

    def __init__(self, lower: Float | None = None, upper: Float | None = None) -> None:
        object.__setattr__(self, "lower", lower)
        object.__setattr__(self, "upper", upper)

Pros

  • Zero construction overhead.
  • Direct int / bool literals in ctor calls (Bounds(lower=0)) are rejected at type-check time.
  • Decimal / Fraction are also rejected at type-check time.
  • Would be rendered in mkdocstrings and become a documented library primitive that other Frequenz projects can adopt.

Cons — this does not actually close the runtime hole

  • The numeric tower still lets int in through widened float variables:
    x: float = 1                    # legal under mypy --strict
    Bounds(lower=x)                 # accepted; runtime stores int
  • Sequence[float] is covariant, so list[int] still satisfies Sequence[Float]:
    raw: Sequence[float] = [1, 2, 3]
    AggregatedMetricValue(avg=1.0, raw=raw)   # accepted; ints stored in raw
  • dataclasses.replace() dispatches through the field annotation (float), not the ctor parameter type — int slips through.
  • raw is stored by reference; the caller can mutate their list after construction and reintroduce int.
  • Structural false positives: any class defining hex() / as_integer_ratio() / is_integer() satisfies Float without being an isinstance of float, and case float(): will reject it.
  • NumPy scalars: numpy.float64 subclasses Python float and is safe, but numpy.float32 / float16 / longdouble are numpy.floating scalars that may satisfy Float structurally without matching case float():.
  • copy / pickle roundtrips preserve values verbatim and cannot repair an already-corrupt instance.

The Protocol closes only the direct-literal case. Every non-trivial ingress path still requires runtime coercion to be safe.

Option 4 — Fast-path runtime coercion

Keep the runtime invariant, but skip the float(x) work when the value is already exactly float:

def __post_init__(self) -> None:
    if type(self.avg) is not float:
        object.__setattr__(self, "avg", float(self.avg))
    ...

Pros

  • Preserves the runtime invariant for every ingress path (same guarantees as Option 1).
  • Common case in production (protobuf converters and correctly-typed callers passing real float) pays only a single type() is float check per field — nanosecond scale.
  • Simple, mechanical change over the Option 1 prototype.

Cons

  • Still not free — still slower than a no-op constructor.
  • raw element check is still O(n) unless we accept caller-list aliasing (see decision points below).
  • Not yet benchmarked in this library.
Option 5 — Live with it: document the trap, warn users

Do not touch the affected classes. Instead:

  • Add explicit warnings to the documentation of every float-typed field: "always pass a real float; the numeric tower may let int slip through your type checker, but doing so may crash accessors that pattern-match on the field type."
  • Adopt Option 2 (case float() | int():) internally as a defensive coding convention for the accessors we control, and document the residual Decimal / Fraction gap as unsupported.
  • Optionally emit a warnings.warn(...) in __post_init__ when a non-float numeric is detected (still pays the isinstance check, but avoids the conversion; can be rate-limited to warn once per class per process).

Pros

  • Zero construction overhead.
  • Honest about the fact that Python has no clean fix; sets accurate expectations for library authors and users.
  • Aligns with how the wider ecosystem handles the same issue (numpy, pandas, etc. all just document that float and int are not always interchangeable).

Cons

  • AssertionError remains a live footgun until the docs land and get read — and docs-based warnings are notoriously ignored.
  • Users passing int will still hit crashes on any accessor with a narrow case float(): arm.
  • Adopting Option 2 internally accepts the residual Decimal / Fraction gap as a known limitation.

Decision points

  1. What is the invariant?

    • "Instances always store real float at runtime" → Option 1 or 4.
    • "The library accepts numeric-tower semantics and expects callers to pass real floats" → Option 2, 3, or 5.

    Every other decision follows from this choice.

  2. Benchmark reality. Options 1 and 4 need real numbers on Python 3.11 and 3.14 with realistic mixed inputs — already-float from protobuf, int from user code, various raw sizes — before committing. The prototype benchmark above is only on int inputs for one class.

  3. raw element policy. If the invariant is "always float", we pay O(n) per construction to verify or coerce every element. If not, we accept aliasing and require callers to hand us a fresh list[float].

  4. NumPy contract. Are numpy.float32 / float16 scalars in scope for our library? If yes, both structural Protocol and match-based accessors need explicit tests.

  5. bool policy. Under Option 1/4, Bounds(lower=True) stores 1.0. Under Options 2/5 it leaks through as True. Under Option 3 it's a type error. Pick one and document it.

Suggested next step

  1. Prototype Option 4 on AggregatedMetricValue, MetricSample, and Bounds and re-run the benchmark. If fast-path coercion is fast enough on realistic inputs, it dominates Options 1 and 3.
  2. If the benchmark still shows unacceptable overhead, decide between Option 3 (accept the runtime holes for the perf win, document the trap) and Option 5 (accept the runtime bug, document the trap).
  3. Whatever we pick, produce user-facing documentation that names the trap explicitly — this issue itself is a good starting point.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority:❓We need to figure out how soon this should be addressedtype:bugSomething isn't working

    Type

    No type

    Fields

    Priority

    None yet

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions