Conversation
There was a problem hiding this comment.
Pull request overview
This PR primarily refactors module imports to use local aliases (improving readability and reducing fully-qualified references) and updates BigDecimal rounding to use a consistently in-place, allocation-minimizing rounding path. It also adjusts tests/benches and adds a convenience Pixi task + script for quickly building/running a BigFloat example file.
Changes:
- Replace many
decimo.<module>fully-qualified references withimport ... as <alias>across Decimal128/BigInt/BigUInt/BigDecimal code. - Rename BigDecimal “round-to-precision” APIs to
*_inplaceand route rounding through in-place BigUInt digit removal. - Add
pixi run bfhelper plus an example/script for quickly building and running a BigFloat-using Mojo file.
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/toml/test_decimo_toml.mojo | Use byte_length() for non-empty string checks. |
| tests/decimal128/test_decimal128_exponential.mojo | Use byte_length() for result string length checks. |
| tests/decimal128/test_decimal128_comparison.mojo | Docstring formatting cleanup. |
| tests/bigdecimal/test_bigdecimal_arithmetics.mojo | Update tests to call round_to_precision_inplace. |
| src/decimo/decimal128/rounding.mojo | Alias-import utility and use alias references. |
| src/decimo/decimal128/exponential.mojo | Alias-import constants/special/utility and use aliases. |
| src/decimo/decimal128/decimal128.mojo | Alias-import submodules and use aliases throughout methods. |
| src/decimo/decimal128/comparison.mojo | Alias-import utility and use alias references. |
| src/decimo/decimal128/arithmetics.mojo | Alias-import utility and use alias references. |
| src/decimo/biguint/exponential.mojo | Alias-import biguint arithmetics. |
| src/decimo/biguint/biguint.mojo | Alias imports; refactor rounding digit-removal wrapper to delegate to in-place implementation. |
| src/decimo/biguint/arithmetics.mojo | Alias-import biguint comparison. |
| src/decimo/bigint10/bigint10.mojo | Alias-import bigint10 modules and decimo.str. |
| src/decimo/bigint10/arithmetics.mojo | Alias-import biguint arithmetics. |
| src/decimo/bigint/exponential.mojo | Alias-import bigint arithmetics. |
| src/decimo/bigint/bigint.mojo | Alias-import bigint submodules and decimo.str. |
| src/decimo/bigdecimal/trigonometric.mojo | Alias-import constants/exponential; switch to round_to_precision_inplace method. |
| src/decimo/bigdecimal/rounding.mojo | Implement round() as copy+inplace; rename public precision-rounder to round_to_precision_inplace. |
| src/decimo/bigdecimal/exponential.mojo | Alias imports; switch internal rounding calls to round_to_precision_inplace. |
| src/decimo/bigdecimal/constants.mojo | Alias-import trigonometric; switch to round_to_precision_inplace. |
| src/decimo/bigdecimal/bigdecimal.mojo | Alias imports; rename round_to_precision method to round_to_precision_inplace. |
| src/decimo/bigdecimal/arithmetics.mojo | Alias-import biguint arithmetics; call round_to_precision_inplace for precision>0 paths. |
| src/cli/calculator/evaluator.mojo | Switch final rounding call to round_to_precision_inplace. |
| pixi.toml | Add bf task to run BigFloat file build+run helper. |
| examples/run_bigfloat.sh | New helper script to build wrapper, package decimo, build & run a single Mojo file. |
| examples/examples_on_bigfloat.mojo | New BigFloat usage example. |
| docs/plans/bigdecimal_enhancement.md | Update plan doc to record T-R2 completion and reformat tables. |
| docs/changelog.md | Expand v0.10.0 changelog narrative and bullets. |
| benches/decimal128/.gitignore | Ignore C# .lscache artifacts. |
| benches/bigdecimal/mojo/bench.mojo | Update bench helper to call round_to_precision_inplace. |
Comments suppressed due to low confidence (1)
src/decimo/bigdecimal/bigdecimal.mojo:2000
- The instance method rename to
round_to_precision_inplaceis also a breaking change for callers using the previousround_to_precisionmethod. If you want to preserve source compatibility, consider adding a thinround_to_precision(...)wrapper method that forwards toround_to_precision_inplace(...)(possibly marked deprecated) while keeping the clearer in-place name.
@always_inline
def round_to_precision_inplace(
mut self,
precision: Int,
rounding_mode: RoundingMode,
remove_extra_digit_due_to_rounding: Bool,
fill_zeros_to_precision: Bool,
) raises:
"""Rounds the number to the specified precision in-place.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
109
to
113
|
|
||
| def round_to_precision( | ||
| def round_to_precision_inplace( | ||
| mut number: BigDecimal, | ||
| precision: Int, | ||
| rounding_mode: RoundingMode, |
| from decimo.rounding_mode import RoundingMode | ||
| from decimo.bigdecimal.exponential import MathCache | ||
| from decimo.bigdecimal.rounding import round_to_precision | ||
| from decimo.bigdecimal.rounding import round_to_precision_inplace |
Comment on lines
+2213
to
2222
| """ | ||
| var result = self.copy() | ||
| result.remove_trailing_digits_with_rounding_inplace( | ||
| ndigits=ndigits, | ||
| rounding_mode=rounding_mode, | ||
| remove_extra_digit_due_to_rounding=( | ||
| remove_extra_digit_due_to_rounding | ||
| ), | ||
| sign=sign, | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR primarily refactors module imports to use local aliases (improving readability and reducing fully-qualified references) and updates BigDecimal rounding to use a consistently in-place, allocation-minimizing rounding path. It also adjusts tests/benches and adds a convenience Pixi task + script for quickly building/running a BigFloat example file.
Changes:
decimo.<module>fully-qualified references withimport ... as <alias>across Decimal128/BigInt/BigUInt/BigDecimal code.*_inplaceand route rounding through in-place BigUInt digit removal.pixi run bfhelper plus an example/script for quickly building and running a BigFloat-using Mojo file.