[blivet:master 1/3] Rewrite of Size.humanReadable() method

Vratislav Podzimek vpodzime at redhat.com
Wed Oct 1 09:32:32 UTC 2014


On Tue, 2014-09-30 at 08:27 -0400, mulhern wrote:
> The observable changes are:
> *) Extremely large values will be displayed using the largest available units,
> rather than as bytes.
> *) The places parameter is eliminated entirely.
> *) A strip parameter is added, with default True. If strip is True, and the
> value is a fractional number, trailing zeros are stripped up to the decimal
> point. strip is True by default to preserve current behavior.
> *) A min_value parameter is added. The default is set to 10, to match the
> current behavior of the method.
> 
> Remark:
> 
> Sizes are now stored in arbitrary precision Decimal values. The base unit
> of computation is bytes, which is something that can be measured exactly.
> Therefore, the quantities that are measured are precisely known, and,
> barring mistakes, all computations should yield precise values.
> Intuitions from other branches of engineering, where there is an
> imprecision in measurement, or from calculating with float types,
> which have limited precision, do not apply here. We do not need to display
> all the precision that is available, but that shouldn't confuse us into
> believing that it is not available.
> 
> Rationale for externally visible changes:
> 
> * Removal of places param.
> Anaconda never sets places at all, and it always specifies max_places with
> a keyword. Openlmi does not appear to use humanReadable at all.
> The use of places was barely tested in the existing unittests and its
> purpose is obscure. Removing it simplifies the code a good deal but does
> not itself change any existing behavior (other than in test code) since
> its default was None.
> 
> * Addition of strip parameter.
> Some users will surely have an engineering background, like myself, and will
> expect the significant digits that they request. strip is set to True,
> however, so that the default behavior is removing 0s, since that is what
> anaconda expects.
> 
> Internal changes and rationale:
> 
> * Obtain fractional digits by using Decimal.quantize(), not round,
> which operates by conversion to float.
> 
> Since Decimals are arbitrary precision, every digit is significant.
> Rounding should occur on the Decimal value, not by conversion to float.
> 
> * Returning prefix unconditionally.
> 
> abbr is set exactly when prefix is set, and they are only set from an
> array which always contains both an abbr and a prefix. So falling back
> on using the prefix when there is no abbreviation makes no sense.
> On the other hand, it is not wrong to keep the prefix value in the array of
> prefixes, it might make sense to add a use_prefix parameter to humanReadable,
> for example.
> 
> * Search only for binary prefixes, not decimal ones.
> Previously, the code searched among decimal prefixes and abbreviations if it
> could not find a binary prefix that worked. As it happens, our binary prefixes
> express a larger range than our decimal prefixes, so no decimal prefix
> should ever have been chosen. Also, humanReadable() should not return
> either binary or decimal prefix, depending on the value of the number
> represented. Two reasons to ditch this behavior and only search for binary
> prefixes.
> Since we are dealing exclusively with binary prefixes we can use
> 1024 as our limit when searching for the correct prefix, not 1000, which
> evidently was supposed to work for both kinds of prefixes.
> 
> * Add an _emptyPrefix constant and use it.
> Previously, very small numbers that should be represented as bytes, w/out
> prefix were handled especially. There was no need for this, so that's
> eliminated.
> 
> * Other more complicated and hard to explain but not effective behavior
> also eliminated.
> The net result is the removal of 23 lines of code, leaving just 17,
> and the addition of some comments making the method overall a good deal
> more readable and parameterizable.
> 
> New method passes all existing tests without any alteration, except for
> a couple tests that used places and now use max_places.
> 
> Signed-off-by: mulhern <amulhern at redhat.com>
> ---
>  blivet/size.py     | 106 ++++++++++++++++++++++++++++-------------------------
>  tests/size_test.py |  60 +++++++++++++++++++++++++++++-
>  2 files changed, 114 insertions(+), 52 deletions(-)
> 
> diff --git a/blivet/size.py b/blivet/size.py
> index e95de6a..8cabe24 100644
> --- a/blivet/size.py
> +++ b/blivet/size.py
> @@ -19,6 +19,7 @@
>  #
>  # Red Hat Author(s): David Cantrell <dcantrell at redhat.com>
>  
> +import itertools
>  import re
>  import string
>  import locale
> @@ -30,7 +31,7 @@ from decimal import ROUND_DOWN
>  import six
>  
>  from .errors import SizePlacesError
> -from .i18n import _, P_, N_
> +from .i18n import _, N_
>  
> 
>  # Container for size unit prefix information
> @@ -59,6 +60,9 @@ _binaryPrefixes = [_Prefix(1024, N_(b"kibi"), N_(b"Ki")),
>                     _Prefix(1024**7, N_(b"zebi"), N_(b"Zi")),
>                     _Prefix(1024**8, N_(b"yobi"), N_(b"Yi"))]
>  
> +# Empty prefix works both for decimal and binary
> +_emptyPrefix = _Prefix(1, "".decode("utf-8"), "".decode("utf-8"))
Any reason for not using u"" here instead of calling decode() on an
old/byte string?

Otherwise this looks good to me.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list