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)

mypy rejecting this code instead of passing and getting a failed assertion at runtime because the actual value type is int, not float and int doesn't inherit from float.

Proposed solution

Annotate the field as float | int and accept the heterogeneity

Stop lying about the annotation: acknowledge in the type system that PEP 484 lets int through, and change every affected field annotation to explicitly permit both. case float() | int(): in accessors is then no longer a "widening workaround" (Option 2) — it is the natural, exhaustive way to unwrap the union, and mypy will actively push every downstream user to handle both branches.

FloatInt: TypeAlias = float | int

@dataclass(frozen=True)
class Bounds:
    lower: FloatInt | None = None
    upper: Floatnt | None = None


@dataclass(frozen=True)
class MetricSample:
    value: Floatnt | AggregatedMetricValue | None

    def as_single_value(self) -> Floatnt | None:
        match self.value:
            case float() | int() as v:
                return v
            case AggregatedMetricValue() as agg:
                return agg.avg
            case None:
                return None
            case unknown:
                assert_never(unknown)  # now genuinely unreachable for numeric-tower reasons

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

We considered many options before arriving to the proposed solution (option 6). They are preserved here for documentation.

All considered alternative 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.

Option 6 — Annotate the field as float | int and accept the heterogeneity

Stop lying about the annotation: acknowledge in the type system that PEP 484 lets int through, and change every affected field annotation to explicitly permit both. case float() | int(): in accessors is then no longer a "widening workaround" (Option 2) — it is the natural, exhaustive way to unwrap the union, and mypy will actively push every downstream user to handle both branches.

@dataclass(frozen=True)
class Bounds:
    lower: float | int | None = None
    upper: float | int | None = None


@dataclass(frozen=True)
class MetricSample:
    value: float | int | AggregatedMetricValue | None

    def as_single_value(self) -> float | int | None:
        match self.value:
            case float() | int() as v:
                return v
            case AggregatedMetricValue() as agg:
                return agg.avg
            case None:
                return None
            case unknown:
                assert_never(unknown)  # now genuinely unreachable for numeric-tower reasons

Pros

  • Completely remove all typing inconsistencies, including def f(g: float) -> None: g.hex(); f(1), which should become def f(g: float | int) -> None: g.hex(); f(1) and now clearly makes mypy and pyright fail (*int has no hex() method).
  • The annotation is honest — it matches what PEP 484 actually admits at runtime. No hidden static-vs-runtime split, no latent AssertionError.
  • It is forward-compatible with a potential change on the typing spec to mean float | int, we would just a have a more verbose type annotation (which can be simplified when/if it is fixed).
  • Zero construction overhead. No object.__setattr__, no Protocol, no coercion.
  • case float() | int(): becomes the natural exhaustive form. assert_never fall-throughs become genuinely unreachable.
  • Trivially compatible with every ingress path — typed callers, untyped callers, dataclasses.replace(), copy, pickle, JSON/dict inflation, protobuf converters. The annotation now admits what these paths already produce.
  • Assignments to float-typed destinations still work (the numeric tower goes the other way too: int is assignable to float), so most downstream code that consumes these fields is unaffected.
  • Turns the fix into a type-checker-enforced migration: any downstream match … case float(): on our fields will trigger a non-exhaustive-match warning under mypy --strict, prompting users to widen to case float() | int(): at their end too.

Cons

  • Needs discipline. We need to remember to always encode float as float | int to not fall in the trap again.
  • Pushes the ambiguity onto every use site. Any downstream code that pattern-matches on the field now has to name int explicitly. Any code that does numeric operations expecting exact float semantics (e.g. .hex(), .as_integer_ratio(), precision-sensitive math) will get mypy errors and need coercion at the call site. This could also be seen as a pro if we consider being honest about how the typing system currently works a good thing.
  • Union noise. Sequence[float] becomes Sequence[float | int]; already-long unions like float | AggregatedMetricValue | None become float | int | AggregatedMetricValue | None. Affects readability of both source and mkdocstrings output. We could provide a FloatInt: TypeAlias = float | int to make this a bit better, with the extra advantage we can clearly document the issue and why the type alias exists in the type alias docstring.
  • type()-based downstream code becomes fragile. Bounds(lower=1) == Bounds(lower=1.0) compares equal (1 == 1.0), but type(Bounds(lower=1).lower) is float is False. Hashing, serialization, and numeric-precision code that keys off the concrete type may break in subtle ways. All of this is inherent to Python (was already an issue when using just float), so I think it is acceptable not getting too clever about this and just forwards to standard Python behavior.
  • Repr / serialization drift. Bounds(lower=1) and Bounds(lower=1.0) render differently. Snapshot tests, log lines, and JSON dumps become sensitive to whether the caller happened to pass an int or a float. This is also fine, in reality most floats will be loaded from protobuf and will be real floats.
  • Broadcasts a Python typing quirk into the public API. Every downstream Frequenz project reading our types sees "this returns int sometimes" and has to reason about a numeric-tower detail that the intent of the field (a real number) does not suggest. This can also be seen as something good. It educates users about Python intricacies.
  • bool still leaks through (since bool ⊂ int), storing True under a numeric annotation. Same decision required as Options 2 / 5. Same as just living with Python shortcomings, instead of getting too clever and fix Python via runtime overhead. It looks like a small risk.

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