docs: Correct tolerance default attribution - #58
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The updated paragraph includes a potentially confusing comparison to polars.Expr.is_close defaults even though Diffly calls Expr.is_close with explicit tolerances.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the tolerances documentation to correctly attribute Diffly’s default float tolerances to Polars testing utilities, preventing readers from misinterpreting how strict Diffly’s default comparisons are.
Changes:
- Reworded the “Default behavior” paragraph to attribute
abs_tol=1e-08/rel_tol=1e-05topolars.testing.assert_frame_equal. - Added an explanatory note contrasting these defaults with Python’s
math.isclosedefaults.
File summaries
| File | Description |
|---|---|
| docs/guides/features/tolerances.ipynb | Corrects documentation text about default tolerance provenance. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Oliver Borchert (borchero)
left a comment
There was a problem hiding this comment.
Thank you!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #58 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 13 13
Lines 1138 1138
=========================================
Hits 1138 1138 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Motivation
The tolerances guide states that the default
abs_tol=1e-08/rel_tol=1e-05match Python'smath.isclosedefaults. They don't —math.isclosedefaults torel_tol=1e-09,abs_tol=0.0. The values actually matchpolars.testing.assert_frame_equal.The error points in the risky direction: a reader trusting the guide would believe diffly is stricter (~9 significant figures) than it is (~5).
Changes
docs/guides/features/tolerances.ipynbto attribute the defaults topolars.testing.assert_frame_equal.Markdown cell only; no cells were executed and no outputs changed. The
compare_framesdocstring note that the implementation mirrorsmath.iscloseis left as-is, since that statement is about the formula and is correct.