Fix Numeric compatibility: hash contract, unit-preserving rounding, finite?/infinite?, to_i/to_f, and more#36
Merged
Conversation
Add doc/numeric-compatibility.md — a full audit of the Numeric (and
common Ruby number-type) API surface against Unit's behaviour.
The document covers:
- Which methods Unit explicitly overrides (and are covered by tests)
- Inherited Numeric methods that work correctly but have no test coverage
(abs2, nonzero?, modulo/%, divmod, between?/clamp, fdiv, integer?,
real/imag/conj/rect, to_int, to_c, clone, +@)
- 8 bugs found during the investigation:
1. #hash violates Ruby's eql?/hash contract (Unit broken as Hash key)
2. #ceil/#floor/#truncate strip the unit; #round does not (inconsistent)
3. #finite?/#infinite? ignore the value (wrong for Float::INFINITY)
4. #positive?/#negative? raise ArgumentError on dimensional units
5. #numerator/#denominator raise despite respond_to? returning true
6. #remainder raises (downstream consequence of bug 2)
7. Numeric#div silently loses unit on incompatible operands
8. #round does not accept the half: keyword (TypeError on all versions)
- Methods that raise by design and should have documented tests
(angle, magnitude, polar, step — vs abs which is overridden correctly)
- Ruby version compatibility (3.3 / 3.4 / 4.0):
- Method sets are identical across all three versions
- Integer#** / Rational#** overflow behaviour changed in Ruby 3.4
(3.3: silently -> Float::INFINITY; 3.4+: stays bignum Integer),
which propagates through Unit#** when value is an Integer
- Kernel#Float() is more permissive in 3.4 (no internal Unit impact)
- Bug 8 (round half:) is pre-existing on all three versions
Unit#eql? compares by value and unit array, but #hash fell through to Object#hash (identity-based). This violated Ruby's eql?/hash contract, silently breaking Hash key lookup and Set membership for Units. Define #hash as [value, unit].hash so any two units that are eql? are guaranteed to produce the same hash code.
These three methods fell through to Numeric's defaults, which call to_i/to_int internally and return a bare number. round was already overridden but with an explicit precision argument that dropped the half: keyword. Use (...) forwarding for all four methods so arguments are forwarded transparently to the underlying value, including half: for round.
The inherited Numeric#finite? always returns true and Numeric#infinite? always returns nil, ignoring the wrapped value entirely. This meant Unit(Float::INFINITY, 'm').finite? returned true and .infinite? returned nil, both wrong. Override both methods to delegate to value when value responds to them (Float does; Integer and Rational do not, so they fall back to the correct defaults: true for finite? and nil for infinite?).
The inherited Numeric#positive? and #negative? call self <=> 0, which raises ArgumentError for dimensional units because comparing metres (or any non-dimensionless unit) against the Integer 0 fails the dimension check in Unit#<=>. Override both methods to delegate directly to value, which already knows how to answer the question without a dimensional comparison.
nertzy
force-pushed
the
numeric-compatibility-docs
branch
from
July 17, 2026 16:25
9920f46 to
a001acd
Compare
Numeric#numerator and Numeric#denominator are both implemented as self.to_r.numerator / self.to_r.denominator. Without to_r, calling either raises NoMethodError even though respond_to? returns true. Follow the Complex precedent: Complex#to_r raises RangeError when the imaginary part is non-zero because it cannot be losslessly represented as a Rational. The same argument applies to dimensional units — a value in metres is not a rational number. Use value.to_r to extract the bare rational without the dimension check. After this change: Unit(3).to_r # => (3/1) Unit(Rational(3,4)).to_r # => (3/4) Unit(3).numerator # => 3 (via Numeric#numerator -> to_r) Unit(3).denominator # => 1 (via Numeric#denominator -> to_r) Unit(3, 'm').to_r # => RangeError: can't convert 3 m into Rational Unit(3, 'm').numerator # => RangeError (same path)
Numeric#remainder computes self - (self/other).truncate * other but internally uses self < 0 via <=>, which raises ArgumentError for dimensional units (comparing metres against Integer 0 fails the dimension check in Unit#<=>). Provide an explicit implementation using the same formula; truncate now preserves the unit (since Bug 2 was fixed), so the arithmetic works out correctly for same-dimension operands.
Bug 7 (div silently losing dimension on incompatible units) was fixed as a side effect of overriding floor to preserve the unit: Numeric#div computes (self / other).floor, so floor now returns a Unit instead of stripping back to a bare integer. Add explicit specs to lock in the correct behaviour for compatible units (dimensionless quotient), incompatible units (combined dimension), and division by a plain scalar (unit preserved).
Cover the previously untested-but-functional surface area of the Numeric API as identified in doc/numeric-compatibility.md: +@, abs2, nonzero?, integer?, real?, real, imag, conj, rect, modulo/%, divmod, fdiv (scalar case), to_int, between?, clamp
Silently stripping the unit from a dimensional quantity (e.g. metres) is unsafe — the caller loses the physical meaning of the number with no indication anything was dropped. Complex is the right precedent: Complex(3,2).to_f raises RangeError: can't convert 3+2i into Float because the imaginary part cannot be represented as a Float. The same argument applies to a dimensional unit. After this change: Unit(3, 'm').to_f # => RangeError: can't convert 3 m into Float Unit(3, 'm').to_i # => RangeError: can't convert 3 m into Integer Unit(3).to_f # => 3.0 (dimensionless: fine) Unit(3).to_i # => 3 Use value.to_f / value.to_i to extract the bare number without the dimension check (e.g. inside Unit itself, or when the caller has already verified the unit is as expected). Two internal callers updated: - approx: now uses @value.to_f directly (it preserves the unit anyway) - fdiv: overridden to use (self/other).approx instead of the inherited Numeric#fdiv which called self.to_f; fdiv now returns a Unit with a float value and the correct combined dimension, fixing the pre-existing bug where fdiv with a Unit divisor returned a wrong dimension.
nertzy
force-pushed
the
numeric-compatibility-docs
branch
from
July 17, 2026 16:37
a001acd to
43e9ec0
Compare
Complex-valued units are physically meaningful — electrical impedance, phasors, and similar quantities are dimensional values with complex scalars. Unit(Complex(3, 4), 'ohm') was already parseable and rendered correctly, but the Numeric methods that distinguish real from complex gave wrong answers because Numeric's defaults unconditionally treat self as real. Override real?, real, imag/imaginary, conj/conjugate, and rect/rectangular to delegate to value. For real-valued units the behaviour is unchanged (real? => true, imag => 0, real => self, conj => self). For complex-valued units: z = Unit(Complex(3, 4), 'ohm') z.real? # => false z.real # => Unit(3, 'ohm') z.imag # => Unit(4, 'ohm') z.conj # => Unit(Complex(3, -4), 'ohm') z.rect # => [Unit(3, 'ohm'), Unit(4, 'ohm')] z.abs # => Unit(5.0, 'ohm') (was already correct) Document Complex-valued units as an explicit design goal in AGENTS.md and add a usage example to README.markdown.
Reflect all changes made since the doc was first written: - hash: note self.class inclusion and why - to_i/to_f: now raise RangeError for dimensional units (not 'strip') - to_r: new entry - numerator/denominator: now via to_r, not direct delegation - fdiv: moved from open issues to fixed - real?/real/imag/conj/rect: moved from inherited Group 1 to overrides table; updated descriptions for complex-value behaviour - to_int: now raises for dimensional units (calls to_i internally) - Complex-valued units: added as explicit design goal section - abs2 for complex values: added to untested list - angle/polar for complex-valued units: added as open issue 14 - Fixed table renumbered: 11 fixed, 3 open
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.
Summary
This PR audits the full
NumericAPI surface againstUnit's behaviour,fixes 11 bugs found in that audit, adds test coverage for previously untested
inherited methods, adds a non-raising query API, and adds a compatibility
survey document.
Bugs fixed
#hash— violated Ruby'seql?/hashcontract. Fixed as[self.class, value, unit].hash;self.classprevents hash bucketcollisions with plain arrays of the same shape.
#ceil,#floor,#truncate— fell through toNumeric's defaultswhich call
to_iinternally, stripping the unit.#roundwas alreadyoverridden but its explicit
precisionarg swallowed thehalf:keyword.All four now use explicit args and preserve the unit.
#finite?/#infinite?— ignored the value.Unit(Float::INFINITY, 'm').infinite?returnednil. Both now delegate tovalue.#positive?/#negative?— raisedArgumentErrorfor dimensionalunits because
Numeric's implementation callsself <=> 0. Both nowdelegate to
value.#to_r— was not defined, so#numeratorand#denominatorraisedNoMethodErrorat runtime despiterespond_to?returningtrue.Following the
Complexprecedent,to_rnow raisesRangeErrorfordimensional units and converts normally for dimensionless ones — giving
numeratoranddenominatorfor free viaNumeric's implementation(
self.to_r.numeratoretc.).#remainder— raised as a downstream consequence of the#truncatestripping bug. Fixed with an explicit implementation:
self - (self / other).truncate * other.#to_i/#to_f— silently stripped the unit from dimensional quantities.Following
Complex#to_f's precedent (raisesRangeErrorwhen imaginary partis non-zero), these now raise for dimensional units. Dimensionless units
convert normally. Two internal callers updated:
approxuses@value.to_fdirectly;
fdivis overridden (see below).#fdiv—Numeric#fdivcalledself.to_f / other, which (a) now raisesfor dimensional units and (b) was already returning wrong dimensions
(
Unit(3,'m').fdiv(Unit(2,'m'))→Unit(1.5 m⁻¹)instead ofUnit(1.5)).Overridden as
(self / other).approx— preserves correct dimension.#real?,#real,#imag,#conj,#rect—Numeric's defaultsunconditionally treat
selfas real (real?alwaystrue,imagalways0, etc.), which is wrong when the scalar value is aComplex. All five nowdelegate to
value. This makes complex-valued units (Unit(Complex(3,4), 'ohm'))a fully supported use case — electrical impedance, phasors, and similar
quantities are dimensional values with complex scalars.
New class methods: non-raising query API
The only way to ask "is this a valid unit string?" was to rescue two exception
classes at the call site (
TypeErrorfromvalidate_unit,Unit::ParseErrorfromparse_unit). Two new class methods handle this:Both accept an optional
systemargument (defaults toUnit.default_system).New test coverage
#hashcontract,Hashkey usage,Setmembership#roundwithhalf:keyword;#ceil,#floor,#truncatewith and withoutndigits#finite?,#infinite?includingFloat::INFINITYcases#positive?,#negative?on dimensional and dimensionless units#to_r,#numerator,#denominator— dimensionless and dimensionalRangeError#to_i,#to_f— dimensionless and dimensionalRangeError#remainderincluding negative dividend/divisor combinations#fdiv— scalar divisor, compatible Unit divisor (dimensionless result), incompatible Unit divisor (combined dimension)#real?,#real,#imag,#conj,#rect— both real-valued and complex-valued unitsUnit.try_parseandUnit.valid?sym: [a, b, ...]): all spellings resolve, first is canonical, prefix composition works on all+@,#abs2,#nonzero?,#integer?,#modulo,#divmod,#div,#to_int,#between?,#clampSurvey document
doc/numeric-compatibility.mddocuments the full audit: which methods Unitoverrides, which inherited methods work correctly, known remaining issues
(
magnitude/absasymmetry,stepraising,angle/polarforcomplex-valued dimensional units), and Ruby version compatibility notes
(3.3 / 3.4 / 4.0 — the method surface is identical but
Integer#**/Rational#**overflow behaviour changed in 3.4).Test plan
bundle exec rake— 242 examples, 0 failures (both DSL and no-DSL runs)