[PATCH 4/5] New method to round a Size to a whole number of a specified unit.
David Lehman
dlehman at redhat.com
Tue Nov 18 14:56:07 UTC 2014
On 11/17/2014 05:41 PM, Anne Mulhern wrote:
>
>
>
>
> ----- Original Message -----
>> From: "David Lehman" <dlehman at redhat.com>
>> To: anaconda-patches at lists.fedorahosted.org
>> Sent: Monday, November 17, 2014 2:36:07 PM
>> Subject: [PATCH 4/5] New method to round a Size to a whole number of a specified unit.
>>
>> ---
>> blivet/size.py | 17 ++++++++++++++++-
>> tests/size_test.py | 40 ++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 56 insertions(+), 1 deletion(-)
>>
>> diff --git a/blivet/size.py b/blivet/size.py
>> index b5ef808..6d24fdf 100644
>> --- a/blivet/size.py
>> +++ b/blivet/size.py
>> @@ -26,13 +26,14 @@ from collections import namedtuple
>>
>> from decimal import Decimal
>> from decimal import InvalidOperation
>> -from decimal import ROUND_DOWN
>> +from decimal import ROUND_DOWN, ROUND_UP, ROUND_HALF_UP
>> import six
>>
>> from .errors import SizePlacesError
>> from .i18n import _, N_
>> from .util import stringize, unicodeize
>>
>> +ROUND_DEFAULT = ROUND_HALF_UP
>
> It would be nice for me if there was a comment here justifying preferring ROUND_HALF_UP to ROUND_HALF_EVEN
> (which is what we get from the default context).
>
> Is there really any benefit to not making the default just consistently ROUND_UP if it is not ROUND_HALF_EVEN?
My understanding is that half-even is mostly beneficial in terms of
being more consistent with negative numbers. Since we're talking about
sizes, we do not care about negative numbers.
To me, half-up is the expected default behavior for rounding. Maybe
others disagree.
>
>>
>> # Container for size unit prefix information
>> _Prefix = namedtuple("Prefix", ["factor", "prefix", "abbr"])
>> @@ -369,3 +370,17 @@ class Size(Decimal):
>>
>> # pylint: disable=undefined-loop-variable
>> return retval_str + " " + _makeSpec(abbr, _BYTES_SYMBOL, xlate,
>> lowercase=False)
>> +
>> + def roundToNearest(self, unit, rounding=ROUND_DEFAULT):
>> + """
>> + :param str unit: a unit specifier
>> + :keyword rounding: which direction to round
>> + :type rounding: one of ROUND_UP, ROUND_DOWN, or ROUND_DEFAULT
>> + :returns: Size rounded to nearest whole specified unit
>> + :rtype: :class:`Size`
>> + """
>> + if rounding not in (ROUND_UP, ROUND_DOWN, ROUND_DEFAULT):
>> + raise ValueError("invalid rounding specifier")
>> +
>> + rounded = self.convertTo(unit).to_integral_value(rounding=rounding)
>> + return Size("%s %s" % (rounded, unit))
>> diff --git a/tests/size_test.py b/tests/size_test.py
>> index 959d8ac..88f0101 100644
>> --- a/tests/size_test.py
>> +++ b/tests/size_test.py
>> @@ -299,6 +299,46 @@ class TranslationTestCase(unittest.TestCase):
>> self.assertTrue(s.humanReadable().endswith("%s%s" % (_("Mi"),
>> _("B"))))
>> self.assertEqual(s.humanReadable(xlate=False), size_str)
>>
>> + def testRoundToNearest(self):
>> + self.assertEqual(size.ROUND_DEFAULT, size.ROUND_HALF_UP)
>> +
>> + s = Size("10.3 GiB")
>> + self.assertEqual(s.roundToNearest("GiB"), Size("10 GiB"))
>> + self.assertEqual(s.roundToNearest("GiB",
>> rounding=size.ROUND_DEFAULT),
>> + Size("10 GiB"))
>> + self.assertEqual(s.roundToNearest("GiB", rounding=size.ROUND_DOWN),
>> + Size("10 GiB"))
>> + self.assertEqual(s.roundToNearest("GiB", rounding=size.ROUND_UP),
>> + Size("11 GiB"))
>> + # >>> Size("10.3 GiB").convertTo("MiB")
>> + # Decimal('10547.19999980926513671875')
>> + self.assertEqual(s.roundToNearest("MiB"), Size("10547 MiB"))
>> + self.assertEqual(s.roundToNearest("MiB", rounding=size.ROUND_UP),
>> + Size("10548 MiB"))
>> + self.assertIsInstance(s.roundToNearest("MiB"), Size)
>> + with self.assertRaises(ValueError):
>> + s.roundToNearest("MiB", rounding='abc')
>> +
>> + # arbitrary decimal rounding constants are not allowed
>> + from decimal import ROUND_HALF_DOWN
>> + with self.assertRaises(ValueError):
>> + s.roundToNearest("MiB", rounding=ROUND_HALF_DOWN)
>> +
>> + s = Size("10.51 GiB")
>> + self.assertEqual(s.roundToNearest("GiB"), Size("11 GiB"))
>> + self.assertEqual(s.roundToNearest("GiB",
>> rounding=size.ROUND_DEFAULT),
>> + Size("11 GiB"))
>> + self.assertEqual(s.roundToNearest("GiB", rounding=size.ROUND_DOWN),
>> + Size("10 GiB"))
>> + self.assertEqual(s.roundToNearest("GiB", rounding=size.ROUND_UP),
>> + Size("11 GiB"))
>> +
>> + s = Size("513 GiB")
>> + self.assertEqual(s.roundToNearest("GiB"), s)
>> + self.assertEqual(s.roundToNearest("TiB"), Size("1 TiB"))
>> + self.assertEqual(s.roundToNearest("TiB", rounding=size.ROUND_DOWN),
>> + Size(0))
>> +
>> class UtilityMethodsTestCase(unittest.TestCase):
>>
>> def testLowerASCII(self):
>> --
>> 1.9.3
>>
>> _______________________________________________
>> anaconda-patches mailing list
>> anaconda-patches at lists.fedorahosted.org
>> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>>
>
> Just one inline comment.
>
> - mulhern
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>
More information about the anaconda-patches
mailing list