Clamp rounded fractional seconds to the field width - #1292
Conversation
DateTimeFormat rounds the fraction to the requested number of digits, so a microsecond value close enough to a whole second rounds up to a value one digit too wide: time(1, 2, 3, 990000) formatted as 'S' gave '10' rather than '9', and 999999 as 'SSSS' gave '10000'. Clamp the rounded value to the largest the field can hold. Rebased onto current master, where the DateTimeFormat tests were moved out of tests/test_dates.py into tests/test_date_time_format.py and flattened from methods into module-level functions; the regression test follows them. Signed-off-by: Vineeth Sai <vineethsai4444@gmail.com>
d4e3c2b to
dbd44a5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1292 +/- ##
=======================================
Coverage 93.37% 93.37%
=======================================
Files 27 27
Lines 4919 4920 +1
=======================================
+ Hits 4593 4594 +1
Misses 326 326
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
According to https://www.unicode.org/reports/tr35/tr35-dates.html#dfst-second, it says Fractional Second (numeric). Truncates, like other numeric time fields, but in this case to the number of digits specified by the field length. So, the The document of Date and Time in Babel, https://babel.pocoo.org/en/latest/dates.html#time-fields, says Fractional second, rounds to the count of letters, and the document should be fixed too. |
|
You are right, and that is the better fix: TR35 says the field truncates, and truncation makes the overflow impossible rather than clamping it after the fact. Two things worth deciding before I switch this PR over, since truncating is a wider change than the overflow repair. It changes three expectations that
The other two cases in that test, and The arithmetic is also better done on the integer microseconds than on the float: return self.format(self.value.microsecond // 10 ** (6 - num), num) # num <= 6
Happy to push that as this PR, updating the three expectations and the docstring and dropping the clamp, if you would rather have the spec-correct behaviour than the narrower overflow fix. Say the word and I will rework it. |
2 participants
DateTimeFormat.format_frac_secondscan render a fractional-seconds field that is wider than its declared number of digits.The method rounds the microseconds to the requested precision:
round(value, num)can carry up to a whole second (for example0.999rounds to1.0). Then1.0 * 10**num == 10**num, which hasnum + 1digits, andself.format('%0*d' % (num, value)) prints all of them. Babel intentionally does not carry into the seconds field, so the fractional field simply overflows its width.A single
Sdigit must be 0 to 9, but it can render10.Fix: clamp the rounded value to the largest in-field value so it stays within the declared width:
This only changes the overflow case (the value is clamped to all nines, the closest representation without carrying into seconds). Every existing
test_fractional_secondsexpectation, including the deliberately rounded['SSSS'] == '0346', is unchanged. Added a regression test covering theS,SS, andSSSSoverflow inputs.