[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