You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Should compare_frames default to pl.Expr.is_close's tolerances rather than polars.testing's? #59
ABS_TOL_DEFAULT = 1e-08 and REL_TOL_DEFAULT = 1e-05 (diffly/_utils.py) are the defaults for compare_frames, the CLI, and diffly.testing.assert_frame_equal.
These match polars.testing.assert_frame_equal (which inherited them from pandas/NumPy). They do not match pl.Expr.is_close, which compare_frames calls under the hood and which defaults to rel_tol=1e-09, abs_tol=0.0 (same as math.isclose, which the compare_frames docstring says the implementation mirrors).
(The tolerances guide currently says the defaults match math.isclose; that's a separate docs fix — PR #58.)
Problem
The polars.testing parity makes sense for diffly.testing.assert_frame_equal — it's a drop-in replacement. But compare_frames is a diff tool, where the cost asymmetry is reversed: a test assertion biases loose to avoid flaky failures, while a diff report should bias toward flagging, since users only inspect what's flagged. With rel_tol=1e-05, a $1 change on a $100,000 value is hidden by default. abs_tol=1e-08 also bakes in a magnitude assumption that a general tool can't justify.
Proposal
Decouple the defaults:
compare_frames and the CLI: rel_tol=1e-09, abs_tol=0.0 — matching pl.Expr.is_close and math.isclose.
diffly.testing.assert_frame_equal: keep 1e-05 / 1e-08 for polars.testing parity.
The tolerances guide's example (differences at the 10th decimal place, ~3e-11 relative) still passes under the stricter defaults.
Trade-offs
abs_tol=0.0 flags near-zero values that differ only by cancellation noise (e.g. 0.0 vs 1e-17). I think that's the correct behavior for a diff tool — surface it, let the user set abs_tol for their data's scale — but it will be visible to some users on upgrade.
compare_frames(a, b).equal() could be False where diffly.testing.assert_frame_equal(a, b) passes. Documenting the two defaults side by side would mitigate this.
It's a behavior change for compare_frames users relying on the current defaults.
If shared defaults are intentional (e.g. so a comparison always explains an assertion failure), a short note in the docs to that effect would resolve this too.
I agree that keeping tolerances synched to polars is best, meaning we should have separate defaults for compare_frames() and testing.assert_frame_equal(). Do you agree? EgeKaraismailogluQCOliver Borchert (@borchero)
One correction to the issue's framing: the reason diffly.testing keeps the looser defaults is not a general claim that assertions should tolerate more, but parity with polars.testing.assert_frame_equal, so adopting diffly doesn't change which tests pass. Implemented in #64.
Activity
MariusMerkleQC commentedon Sep 7, 2026
I agree that keeping tolerances synched to
polarsis best, meaning we should have separate defaults forcompare_frames()andtesting.assert_frame_equal(). Do you agree? EgeKaraismailogluQC Oliver Borchert (@borchero)borchero commentedon Sep 8, 2026
Seems reasonable to me
MariusMerkleQC commentedon Sep 9, 2026
Would you like to give it a shot? Hayden Anderson (@haydena7)
haydena7 commentedon Sep 9, 2026
Marius Merkle (@MariusMerkleQC) Sure!
haydena7 commentedon Sep 9, 2026
One correction to the issue's framing: the reason
diffly.testingkeeps the looser defaults is not a general claim that assertions should tolerate more, but parity withpolars.testing.assert_frame_equal, so adoptingdifflydoesn't change which tests pass. Implemented in #64.