Correct IFDRational.__float__() return value#9676
Open
nyxst4ck wants to merge 1 commit into
Open
Conversation
IFDRational delegates __eq__, __int__, __round__, __repr__ and arithmetic to self._val (the normalized Fraction), but never defined __float__. For a non-integral numerator (e.g. IFDRational(1.5, 3), whose value is 0.5) float() fell back to numbers.Rational.__float__, which computes int(numerator) / int(denominator) = int(1.5) / int(3) = 0.333..., disagreeing with ==, int() and repr() (all 0.5). Delegate __float__ to self._val so float() is consistent with the rest of the class.
radarhere
reviewed
Jun 16, 2026
Comment on lines
+44
to
+46
| # normalized fraction. For a non-integral numerator (1.5 / 3 == 0.5) the | ||
| # inherited Rational.__float__ used int(numerator) / int(denominator) and | ||
| # returned 1/3 instead. |
Member
There was a problem hiding this comment.
Suggested change
| # normalized fraction. For a non-integral numerator (1.5 / 3 == 0.5) the | |
| # inherited Rational.__float__ used int(numerator) / int(denominator) and | |
| # returned 1/3 instead. | |
| # normalized fraction |
I don't think we need to document prior incorrect behaviour.
radarhere
reviewed
Jun 16, 2026
| assert r == 0.5 | ||
| assert float(r) == 0.5 | ||
|
|
||
| # Integral and 0/0 (nan) cases stay correct. |
Member
There was a problem hiding this comment.
Suggested change
| # Integral and 0/0 (nan) cases stay correct. | |
| # Integer numerator and 0/0 (nan) cases |
radarhere
approved these changes
Jun 17, 2026
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.
Changes proposed
IFDRational.__float__returns the wrong value for a non-integral numerator.IFDRationaldelegates__eq__,__int__,__round__,__repr__and all arithmetic toself._val(the normalizedFraction), but never defined__float__. Sofloat()falls back tonumbers.Rational.__float__, which doesint(self.numerator) / int(self.denominator). For a non-integral numerator the stored_numerator/_denominatorare un-normalized, so the result disagrees with every other accessor:This is reachable via the public
IFDRational(value, denominator)constructor.Fix
Delegate
__float__toself._val, the same way__int__,__round__,__repr__and__eq__are delegated, sofloat()is consistent with the normalized fraction.Tests
Added
test_floattoTests/test_tiff_ifdrational.py: it assertsfloat(IFDRational(1.5, 3)) == 0.5, and that the integral and0/0(nan) cases stay correct. It fails onmain(0.333… != 0.5) and passes with the change;Tests/test_tiff_ifdrational.pyis green (6 passed).ruffandblackclean.