fix: carry filesize suffix when mantissa rounds up to base#600
Open
patchwright wants to merge 1 commit into
Open
fix: carry filesize suffix when mantissa rounds up to base#600patchwright wants to merge 1 commit into
patchwright wants to merge 1 commit into
Conversation
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.
Problem
All three public functions (
traditional,binary,decimal) can produce impossible output like"1,024.0 KB"instead of"1.0 MB"for values just below a unit boundary.Root cause:
_to_strpicks the suffix from the unrounded value, then formats the mantissa — which can round up tobase, producing an impossible result.Fix
Convert
suffixesto a list before the loop. After computing the mantissa, round and compare againstbase. If the rounded value hitsbase, step up one suffix and recompute:Math: when the current unit is
base**i, dividingsizeby that sameunitgives the value in the next suffix (whose denominator isbase**(i+1), which divides intobase * size / base**(i+1) = size / base**i = size / unit).Also removes the existing TODO comment that flagged this exact issue.
Tests
Added
test_rollover_decimal,test_rollover_traditional, andtest_rollover_binarycovering the unit boundaries for all three functions.