[PATCH] Do not return None from Size.__str__ (#869405)

[Vratislav Podzimek]

If no value is < 1000, return the value in YBs.

Signed-off-by: Vratislav Podzimek 
---
 pyanaconda/storage/size.py | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/pyanaconda/storage/size.py b/pyanaconda/storage/size.py
index 757c97b..25247fc 100644
--- a/pyanaconda/storage/size.py
+++ b/pyanaconda/storage/size.py
@@ -208,27 +208,29 @@ class Size(Decimal):
         if Decimal(check) < 1000:
             return "%s B" % check

-        for factor, prefix, abbr in _prefixes:
+        for item in _prefixes:
+            factor, prefix, abbr = item
             newcheck = super(Size, self).__div__(Decimal(factor))

if newcheck < 1000:
-                if places is not None:
-                    fmt = "%%.%df" % places
-                    retval = fmt % newcheck
-                else:
-                    retval = self._trimEnd("%f" % newcheck)
-
-                if max_places is not None:
-                    (whole, point, fraction) = retval.partition(".")
-                    if point and len(fraction) > max_places:
-                        if max_places == 0:
-                            retval = whole
-                        else:
-                            retval = "%s.%s" % (whole, fraction[:max_places])
-
-                if abbr:
-                    return retval + " " + abbr + _("B")
+                # nice value, use this factor, prefix and abbr
+                break
+
+        if places is not None:
+            fmt = "%%.%df" % places
+            retval = fmt % newcheck
+        else:
+            retval = self._trimEnd("%f" % newcheck)
+
+        if max_places is not None:
+            (whole, point, fraction) = retval.partition(".")
+            if point and len(fraction) > max_places:
+                if max_places == 0:
+                    retval = whole
                 else:
-                    return retval + " " + prefix + P_("byte", "bytes", newcheck)
+                    retval = "%s.%s" % (whole, fraction[:max_places])

-        return None
+        if abbr:
+            return retval + " " + abbr + _("B")
+        else:
+            return retval + " " + prefix + P_("byte", "bytes", newcheck)

[Brian C. Lane]

On Mon, Oct 29, 2012 at 08:15:28PM +0100, Vratislav Podzimek wrote:
> If no value is < 1000, return the value in YBs.
> Signed-off-by: Vratislav Podzimek
>
> pyanaconda/storage/size.py | 40 +++++++++++++++++++++-------------------
>  1 file changed, 21 insertions(+), 19 deletions(-)
>
> diff --git a/pyanaconda/storage/size.py b/pyanaconda/storage/size.py
> index 757c97b..25247fc 100644
> --- a/pyanaconda/storage/size.py
> +++ b/pyanaconda/storage/size.py
> @@ -208,27 +208,29 @@ class Size(Decimal):
>          if Decimal(check) < 1000:
>              return "%s B" % check
>    for factor, prefix, abbr in _prefixes:
>    for item in _prefixes:
>        factor, prefix, abbr = item

Looks ok, except I'm wondering why you factored the tuple unpacking out
of the for loop.

[Vratislav Podzimek]

On Tue, 2012-10-30 at 08:35 -0700, Brian C. Lane wrote:
> On Mon, Oct 29, 2012 at 08:15:28PM +0100, Vratislav Podzimek wrote:
> > If no value is < 1000, return the value in YBs.
> > Signed-off-by: Vratislav Podzimek
> >
> > pyanaconda/storage/size.py | 40 +++++++++++++++++++++-------------------
> >  1 file changed, 21 insertions(+), 19 deletions(-)
> >
> > diff --git a/pyanaconda/storage/size.py b/pyanaconda/storage/size.py
> > index 757c97b..25247fc 100644
> > --- a/pyanaconda/storage/size.py
> > +++ b/pyanaconda/storage/size.py
> > @@ -208,27 +208,29 @@ class Size(Decimal):
> >          if Decimal(check) < 1000:
> >              return "%s B" % check
> >    for factor, prefix, abbr in _prefixes:
> >    for item in _prefixes:
> >        factor, prefix, abbr = item
>
> Looks ok, except I'm wondering why you factored the tuple unpacking out
> of the for loop.

Because it looked to me like a bug. It of course works either way, but
using the substitute variable from the for-cycle outside of it was
something that alarmed my "good-programming-habits" senses. I can change
it if nobody else has the same problem as I have.

[Brian C. Lane]

On Wed, Oct 31, 2012 at 09:32:10AM +0100, Vratislav Podzimek wrote:
> On Tue, 2012-10-30 at 08:35 -0700, Brian C. Lane wrote:
> > On Mon, Oct 29, 2012 at 08:15:28PM +0100, Vratislav Podzimek wrote:
> > > If no value is < 1000, return the value in YBs.
> > > Signed-off-by: Vratislav Podzimek
> > >
> > > pyanaconda/storage/size.py | 40 +++++++++++++++++++++-------------------
> > >  1 file changed, 21 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/pyanaconda/storage/size.py b/pyanaconda/storage/size.py
> > > index 757c97b..25247fc 100644
> > > --- a/pyanaconda/storage/size.py
> > > +++ b/pyanaconda/storage/size.py
> > > @@ -208,27 +208,29 @@ class Size(Decimal):
> > >          if Decimal(check) < 1000:
> > >              return "%s B" % check
> > >    for factor, prefix, abbr in _prefixes:
> > >    for item in _prefixes:
> > >        factor, prefix, abbr = item
> >
> > Looks ok, except I'm wondering why you factored the tuple unpacking out
> > of the for loop.
>
> Because it looked to me like a bug. It of course works either way, but
> using the substitute variable from the for-cycle outside of it was
> something that alarmed my "good-programming-habits" senses. I can change
> it if nobody else has the same problem as I have.

I'd leave it the way it was. Both because I like it, and because it
means less change.