Conversation
There was a problem hiding this comment.
Pull request overview
Adds missing Python decimal.Decimal-compat features to decimo, specifically BigDecimal.same_quantum() and a new ROUND_HALF_DOWN rounding mode.
Changes:
- Add
BigDecimal.same_quantum(other)(scale/exponent equality check). - Introduce
ROUND_HALF_DOWN/RoundingMode.half_down()and implement its rounding behavior in BigUInt digit-truncation rounding. - Extend tests and roadmap docs to cover the new API surface.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/bigdecimal/test_data/bigdecimal_rounding.toml | Adds HALF_DOWN rounding test cases (data-driven). |
| tests/bigdecimal/test_bigdecimal_rounding.mojo | Adds Python parity tests for ROUND_HALF_DOWN. |
| tests/bigdecimal/test_bigdecimal_methods.mojo | Adds unit tests for BigDecimal.same_quantum(). |
| src/decimo/rounding_mode.mojo | Adds ROUND_HALF_DOWN alias and RoundingMode.half_down(). |
| src/decimo/biguint/biguint.mojo | Implements HALF_DOWN in remove_trailing_digits_with_rounding(). |
| src/decimo/bigdecimal/rounding.mojo | Documents HALF_DOWN in BigDecimal rounding helpers (one docstring formatting issue noted). |
| src/decimo/bigdecimal/bigdecimal.mojo | Adds same_quantum() method to BigDecimal. |
| src/decimo/init.mojo | Exports ROUND_HALF_DOWN from the package entrypoint. |
| docs/plans/api_roadmap.md | Marks same_quantum and ROUND_HALF_DOWN as implemented. |
Comments suppressed due to low confidence (1)
src/decimo/bigdecimal/rounding.mojo:132
- The
round_to_precision()docstring rounding-mode list was accidentally collapsed into a single line (HALF_UP + HALF_DOWN + HALF_EVEN), which makes the documentation hard to read and looks like a formatting mistake. Please split these back into separate, properly indented lines (and include HALF_DOWN as its own entry) to match the style used inround()above.
RoundingMode.ROUND_DOWN: Round toward zero.
RoundingMode.ROUND_UP: Round away from zero.
RoundingMode.ROUND_HALF_UP: Round half away from zero. RoundingMode.ROUND_HALF_DOWN: Round half toward zero. RoundingMode.ROUND_HALF_EVEN: Round half to even (banker's).
RoundingMode.ROUND_CEILING: Round toward +∞.
RoundingMode.ROUND_FLOOR: Round toward -∞.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """Round toward zero if <= 0.5.""" | ||
| return Self(6) | ||
|
|
There was a problem hiding this comment.
RoundingMode.half_down() is added here, but at least one rounding implementation (e.g. src/decimo/decimal128/utility.mojo round_to_keep_first_n_digits) uses an else branch that effectively treats any unhandled mode as HALF_EVEN. After this change, passing RoundingMode.half_down() into Decimal128 rounding will produce HALF_EVEN results instead of HALF_DOWN. Please update those call sites to explicitly handle HALF_DOWN (and ideally make HALF_EVEN an explicit elif, with an error for truly unknown modes).
| """Round toward zero if <= 0.5.""" | |
| return Self(6) | |
| """ | |
| Round toward zero if <= 0.5. | |
| Note: | |
| Currently, HALF_DOWN is mapped to the same internal value as HALF_EVEN | |
| to maintain compatibility with existing rounding implementations that do | |
| not yet explicitly handle HALF_DOWN. This means that, until those | |
| implementations are updated, ROUND_HALF_DOWN will behave identically to | |
| ROUND_HALF_EVEN. | |
| """ | |
| return Self(2) |
same_quantum() + ROUND_HALF_DOWN rounding mode…e improvement (#177) Related to Issue #175. This PR adds three new features: 1. `same_quantum(other)` on `BigDecimal` Returns `True` if both operands have the same scale (exponent), matching Python's `Decimal.same_quantum()`. 2. `ROUND_HALF_DOWN` rounding mode Adds the 7th rounding mode: ties round toward zero (opposite of `ROUND_HALF_UP`). 3. Rounding architecture improvement BigUInt.remove_trailing_digits_with_rounding() and round_to_keep_first_n_digits() (Decimal128) now accept a sign: Bool parameter and handle CEILING/FLOOR → UP/DOWN translation internally Removed duplicated CEILING/FLOOR translation from 3 call sites. The else fallback in `Decimal128`'s `round_to_keep_first_n_digits` no longer silently treats unknown modes as `HALF_EVEN` — it is now a proper debug_assert for unknown modes. Updated all rounding docstrings to list all 7 modes consistently New `RoundingMode.half_down()` constructor and `ROUND_HALF_DOWN` comptime alias Rounding logic in `BigUInt.remove_trailing_digits_with_rounding()`: rounds up only when cut-off `digit > 5`, or exactly 5 with non-zero digits beyond
Related to Issue #175.
This PR adds three new features:
same_quantum(other)onBigDecimalReturns
Trueif both operands have the same scale (exponent), matching Python'sDecimal.same_quantum().ROUND_HALF_DOWNrounding modeAdds the 7th rounding mode: ties round toward zero (opposite of
ROUND_HALF_UP).Rounding architecture improvement
BigUInt.remove_trailing_digits_with_rounding() and round_to_keep_first_n_digits() (Decimal128) now accept a sign: Bool parameter and handle CEILING/FLOOR → UP/DOWN translation internally
Removed duplicated CEILING/FLOOR translation from 3 call sites. The else fallback in
Decimal128'sround_to_keep_first_n_digitsno longer silently treats unknown modes asHALF_EVEN— it is now a proper debug_assert for unknown modes. Updated all rounding docstrings to list all 7 modes consistentlyNew
RoundingMode.half_down()constructor andROUND_HALF_DOWNcomptime alias Rounding logic inBigUInt.remove_trailing_digits_with_rounding(): rounds up only when cut-offdigit > 5, or exactly 5 with non-zero digits beyond