fix: Number ordering honors precision/imaginary_unit/case_insensitive#17
Merged
Conversation
Number.__make_comparison built its inner Solver with no params, falling back to the default precision=24. Two Numbers built with precision=256 that differed only beyond the 24th digit were compared as if they were equal because their values truncated at parse time. Pass **self.params through so the comparison runs at the user's configured precision (and preserves imaginary_unit / case_insensitive). Regression test in tests/test_number_comparison_precision.py covers <, >, <=, >= at precision=256 against a Number that differs at the 52nd digit. See ai/improvements_2026-05-09.md item #3. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
hozblok
added a commit
that referenced
this pull request
May 12, 2026
…cept numeric types, add reverse arithmetic
BREAKING CHANGE: rework of the Number constructor and the four
arithmetic surfaces (forward, reverse, comparison, hashing). Four
pieces, all in src/formula/formula.py:
1. Number.__init__ no longer takes `imaginary_unit` or `case_insensitive`.
Numbers built from Number always use the default complex unit "i"
and case-sensitive parsing. Internal Solvers (in __make_operation,
__make_comparison, __prepare_comparison, __abs__) receive only
`precision` via self.params. Solver itself still accepts the two
kwargs — this change is Number-only.
2. Number(expression) now accepts Union[Number, str, int, float]:
Number(5) # -> Number with expression "5"
Number(3.14) # -> Number with expression "3.14"
Number(Number("x")) # -> unwraps the inner expression
bool is rejected explicitly with a TypeError naming "bool" (without
that check, Number(True) would silently become Number("True")
because bool is an int subclass). Anything else (None, list, dict,
arbitrary objects) raises TypeError naming the offending type and
the accepted set.
3. The reverse-arithmetic dunders (__radd__/__rsub__/__rmul__/
__rtruediv__/__rpow__) drop the prior `Number(str(__value), ...)`
workaround — the constructor now handles type coercion itself,
so `Number(__value, precision=self._precision)` is correct and
reads cleanly. Forward arithmetic and comparison helpers drop the
conditional `__value.expression if isinstance ... else str(...)`
for the same reason.
4. Public type hints on every arithmetic and comparison method widen
from Union[Number, str] to Union[Number, str, int, float] to match
what the constructor now accepts.
Migration:
# Before
Number("1+1*j", imaginary_unit="j")
Number("X+Y", case_insensitive=True)
# After — pre-process the formula text yourself or use Solver
# directly when those flags matter:
Solver("1+1*j", imaginary_unit="j")(...)
Solver("X+Y", case_insensitive=True)(...)
Suggested for the 5.0 milestone alongside #16 (Solver dict-only).
Coordination flag: PR fix/03-number-comparison-honors-params (#17)
currently advertises that Number comparisons honor imaginary_unit and
case_insensitive. After this lands, that part of its premise is gone —
the precision portion of #17 is still real and still wanted. Title
and body of #17 should be scoped down before merge.
Regression coverage:
tests/test_number_constructor_types.py (new) — 10 cases:
int, negative int, float, str (still works), Number unwrap with
independent precision, bool rejected, None rejected, list rejected,
dict rejected, generic object rejected with type-name in message.
tests/test_number_reverse_arithmetic.py — unchanged from the prior
commit on this branch; still passes against the new constructor.
Full pytest suite: 333/333 passing.
See ai/improvements_2026-05-09.md item #4 and CLAUDE.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
hozblok
added a commit
that referenced
this pull request
May 12, 2026
…er review) Applies owner review on PR #18: - "Remove Number from this Union. Support only primitives." (line 110) - "Remove this lines." (line 116) — the Number-input branch in _coerce - "Same around Number here" (line 133) — drop the Number-coercion in __make_operation / __prepare_comparison; route through __value._value when __value is a Number, str() it inline when it's a primitive - "Add self._value = Solver(...)()" (line 137) — eagerly evaluate the expression at construction; cache as the canonical value used by arithmetic and comparisons Design pivot. Pre-review, Number was an abstract wrapper that could hold either a literal numeric expression or a symbolic one (with unbound variables, or with arbitrary deferred composition). Post-review, Number represents a *concrete numeric value*. The expression is evaluated by Solver once, at __init__, and the result is cached on self._value. self._expression keeps the original input for round-tripping / debug. Arithmetic and comparison helpers use self._value as the source of truth. Unbound-variable handling: Solver() raises ValueError when called with no bindings on an expression that has variables. Number.__init__ catches that and re-raises as ValueError with a message that names the offending input and the constraint — preserving the original via `raise ... from e` so the Solver-level detail is still reachable through __cause__. Constructor input set narrows to (str, int, float). Number-as-input is gone. bool stays rejected explicitly (int subclass). Tests updated: - tests/test_number_constructor_types.py: drop the inner-Number unwrap tests; add tests for unbound-variable rejection, garbage expression rejection, error-chain preservation, and explicit Number-rejection at both __init__ and _coerce_expression entry points; widen direct-method tests for str inputs. - Existing tests in test_number_reverse_arithmetic.py and the Number cases in test_simple.py still pass — they only used literal numeric or complex expressions, all of which evaluate to concrete values. Coordination: PR #23 (symbolic-accumulation direction) is now in direct conflict with this design and should be closed in favor of this approach. PR #18's body will be updated to flag that alongside the existing PR #17 coordination note. Full pytest suite: 340/340. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
hozblok
added a commit
that referenced
this pull request
May 19, 2026
…cept numeric types, add reverse arithmetic
BREAKING CHANGE: rework of the Number constructor and the four
arithmetic surfaces (forward, reverse, comparison, hashing). Four
pieces, all in src/formula/formula.py:
1. Number.__init__ no longer takes `imaginary_unit` or `case_insensitive`.
Numbers built from Number always use the default complex unit "i"
and case-sensitive parsing. Internal Solvers (in __make_operation,
__make_comparison, __prepare_comparison, __abs__) receive only
`precision` via self.params. Solver itself still accepts the two
kwargs — this change is Number-only.
2. Number(expression) now accepts Union[Number, str, int, float]:
Number(5) # -> Number with expression "5"
Number(3.14) # -> Number with expression "3.14"
Number(Number("x")) # -> unwraps the inner expression
bool is rejected explicitly with a TypeError naming "bool" (without
that check, Number(True) would silently become Number("True")
because bool is an int subclass). Anything else (None, list, dict,
arbitrary objects) raises TypeError naming the offending type and
the accepted set.
3. The reverse-arithmetic dunders (__radd__/__rsub__/__rmul__/
__rtruediv__/__rpow__) drop the prior `Number(str(__value), ...)`
workaround — the constructor now handles type coercion itself,
so `Number(__value, precision=self._precision)` is correct and
reads cleanly. Forward arithmetic and comparison helpers drop the
conditional `__value.expression if isinstance ... else str(...)`
for the same reason.
4. Public type hints on every arithmetic and comparison method widen
from Union[Number, str] to Union[Number, str, int, float] to match
what the constructor now accepts.
Migration:
# Before
Number("1+1*j", imaginary_unit="j")
Number("X+Y", case_insensitive=True)
# After — pre-process the formula text yourself or use Solver
# directly when those flags matter:
Solver("1+1*j", imaginary_unit="j")(...)
Solver("X+Y", case_insensitive=True)(...)
Suggested for the 5.0 milestone alongside #16 (Solver dict-only).
Coordination flag: PR fix/03-number-comparison-honors-params (#17)
currently advertises that Number comparisons honor imaginary_unit and
case_insensitive. After this lands, that part of its premise is gone —
the precision portion of #17 is still real and still wanted. Title
and body of #17 should be scoped down before merge.
Regression coverage:
tests/test_number_constructor_types.py (new) — 10 cases:
int, negative int, float, str (still works), Number unwrap with
independent precision, bool rejected, None rejected, list rejected,
dict rejected, generic object rejected with type-name in message.
tests/test_number_reverse_arithmetic.py — unchanged from the prior
commit on this branch; still passes against the new constructor.
Full pytest suite: 333/333 passing.
See ai/improvements_2026-05-09.md item #4 and CLAUDE.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
hozblok
added a commit
that referenced
this pull request
May 19, 2026
…er review) Applies owner review on PR #18: - "Remove Number from this Union. Support only primitives." (line 110) - "Remove this lines." (line 116) — the Number-input branch in _coerce - "Same around Number here" (line 133) — drop the Number-coercion in __make_operation / __prepare_comparison; route through __value._value when __value is a Number, str() it inline when it's a primitive - "Add self._value = Solver(...)()" (line 137) — eagerly evaluate the expression at construction; cache as the canonical value used by arithmetic and comparisons Design pivot. Pre-review, Number was an abstract wrapper that could hold either a literal numeric expression or a symbolic one (with unbound variables, or with arbitrary deferred composition). Post-review, Number represents a *concrete numeric value*. The expression is evaluated by Solver once, at __init__, and the result is cached on self._value. self._expression keeps the original input for round-tripping / debug. Arithmetic and comparison helpers use self._value as the source of truth. Unbound-variable handling: Solver() raises ValueError when called with no bindings on an expression that has variables. Number.__init__ catches that and re-raises as ValueError with a message that names the offending input and the constraint — preserving the original via `raise ... from e` so the Solver-level detail is still reachable through __cause__. Constructor input set narrows to (str, int, float). Number-as-input is gone. bool stays rejected explicitly (int subclass). Tests updated: - tests/test_number_constructor_types.py: drop the inner-Number unwrap tests; add tests for unbound-variable rejection, garbage expression rejection, error-chain preservation, and explicit Number-rejection at both __init__ and _coerce_expression entry points; widen direct-method tests for str inputs. - Existing tests in test_number_reverse_arithmetic.py and the Number cases in test_simple.py still pass — they only used literal numeric or complex expressions, all of which evaluate to concrete values. Coordination: PR #23 (symbolic-accumulation direction) is now in direct conflict with this design and should be closed in favor of this approach. PR #18's body will be updated to flag that alongside the existing PR #17 coordination note. Full pytest suite: 340/340. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
hozblok
added a commit
that referenced
this pull request
May 19, 2026
…cept numeric types, add reverse arithmetic
BREAKING CHANGE: rework of the Number constructor and the four
arithmetic surfaces (forward, reverse, comparison, hashing). Four
pieces, all in src/formula/formula.py:
1. Number.__init__ no longer takes `imaginary_unit` or `case_insensitive`.
Numbers built from Number always use the default complex unit "i"
and case-sensitive parsing. Internal Solvers (in __make_operation,
__make_comparison, __prepare_comparison, __abs__) receive only
`precision` via self.params. Solver itself still accepts the two
kwargs — this change is Number-only.
2. Number(expression) now accepts Union[Number, str, int, float]:
Number(5) # -> Number with expression "5"
Number(3.14) # -> Number with expression "3.14"
Number(Number("x")) # -> unwraps the inner expression
bool is rejected explicitly with a TypeError naming "bool" (without
that check, Number(True) would silently become Number("True")
because bool is an int subclass). Anything else (None, list, dict,
arbitrary objects) raises TypeError naming the offending type and
the accepted set.
3. The reverse-arithmetic dunders (__radd__/__rsub__/__rmul__/
__rtruediv__/__rpow__) drop the prior `Number(str(__value), ...)`
workaround — the constructor now handles type coercion itself,
so `Number(__value, precision=self._precision)` is correct and
reads cleanly. Forward arithmetic and comparison helpers drop the
conditional `__value.expression if isinstance ... else str(...)`
for the same reason.
4. Public type hints on every arithmetic and comparison method widen
from Union[Number, str] to Union[Number, str, int, float] to match
what the constructor now accepts.
Migration:
# Before
Number("1+1*j", imaginary_unit="j")
Number("X+Y", case_insensitive=True)
# After — pre-process the formula text yourself or use Solver
# directly when those flags matter:
Solver("1+1*j", imaginary_unit="j")(...)
Solver("X+Y", case_insensitive=True)(...)
Suggested for the 5.0 milestone alongside #16 (Solver dict-only).
Coordination flag: PR fix/03-number-comparison-honors-params (#17)
currently advertises that Number comparisons honor imaginary_unit and
case_insensitive. After this lands, that part of its premise is gone —
the precision portion of #17 is still real and still wanted. Title
and body of #17 should be scoped down before merge.
Regression coverage:
tests/test_number_constructor_types.py (new) — 10 cases:
int, negative int, float, str (still works), Number unwrap with
independent precision, bool rejected, None rejected, list rejected,
dict rejected, generic object rejected with type-name in message.
tests/test_number_reverse_arithmetic.py — unchanged from the prior
commit on this branch; still passes against the new constructor.
Full pytest suite: 333/333 passing.
See ai/improvements_2026-05-09.md item #4 and CLAUDE.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
hozblok
added a commit
that referenced
this pull request
May 19, 2026
…er review) Applies owner review on PR #18: - "Remove Number from this Union. Support only primitives." (line 110) - "Remove this lines." (line 116) — the Number-input branch in _coerce - "Same around Number here" (line 133) — drop the Number-coercion in __make_operation / __prepare_comparison; route through __value._value when __value is a Number, str() it inline when it's a primitive - "Add self._value = Solver(...)()" (line 137) — eagerly evaluate the expression at construction; cache as the canonical value used by arithmetic and comparisons Design pivot. Pre-review, Number was an abstract wrapper that could hold either a literal numeric expression or a symbolic one (with unbound variables, or with arbitrary deferred composition). Post-review, Number represents a *concrete numeric value*. The expression is evaluated by Solver once, at __init__, and the result is cached on self._value. self._expression keeps the original input for round-tripping / debug. Arithmetic and comparison helpers use self._value as the source of truth. Unbound-variable handling: Solver() raises ValueError when called with no bindings on an expression that has variables. Number.__init__ catches that and re-raises as ValueError with a message that names the offending input and the constraint — preserving the original via `raise ... from e` so the Solver-level detail is still reachable through __cause__. Constructor input set narrows to (str, int, float). Number-as-input is gone. bool stays rejected explicitly (int subclass). Tests updated: - tests/test_number_constructor_types.py: drop the inner-Number unwrap tests; add tests for unbound-variable rejection, garbage expression rejection, error-chain preservation, and explicit Number-rejection at both __init__ and _coerce_expression entry points; widen direct-method tests for str inputs. - Existing tests in test_number_reverse_arithmetic.py and the Number cases in test_simple.py still pass — they only used literal numeric or complex expressions, all of which evaluate to concrete values. Coordination: PR #23 (symbolic-accumulation direction) is now in direct conflict with this design and should be closed in favor of this approach. PR #18's body will be updated to flag that alongside the existing PR #17 coordination note. Full pytest suite: 340/340. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
hozblok
added a commit
that referenced
this pull request
Jun 2, 2026
Static metadata (name, description, classifiers, license, authors, urls,
requires-python, optional-dependencies, package layout) now lives in
pyproject.toml's [project] table per PEP 621. setup.py is now a thin
imperative shim: it still owns the version-string read (kept as a single
source of truth, also feeding the VERSION_INFO macro on the C++ side)
and the Pybind11Extension wiring.
This change makes the package legible to modern tooling — uv, pip,
hatch's project introspection, dependency scanners, IDE indexers — that
parse pyproject.toml without invoking setup.py.
Verified by rebuilding the extension, running the full pytest suite
(316/316), and inspecting `importlib.metadata.metadata('formula')` on
the installed package — name, version, summary, license, author email,
classifiers all carry through correctly.
See ai/improvements_2026-05-09.md item #17.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Problem.
Number.__make_comparisonbuilt its innerSolverwith no params, falling back toprecision=24. TwoNumber(..., precision=256)values that differed only beyond the 24th digit were compared as if equal because their numeric values truncated at parse time.imaginary_unitandcase_insensitivewere also silently dropped.Fix.
src/formula/formula.py:145— forward**self.paramsto the comparison Solver, matching what__make_operationand__prepare_comparisonalready do.Test. New file
tests/test_number_comparison_precision.pyexercises<,>,<=,>=atprecision=256against a Number that differs at the 52nd digit. All pass; full suite 319/319.