Re: [gentoo-portage-dev] [PATCH 1/2] Use a localized size formatting function and ISO/IEC prefixes.

2014-03-30 Thread Alec Warner
On Sun, Mar 30, 2014 at 12:56 AM, Michał Górny  wrote:

> Dnia 2014-03-30, o godz. 00:03:02
> Alexander Berntsen  napisał(a):
>
> > -BEGIN PGP SIGNED MESSAGE-
> > Hash: SHA256
> >
> > Please use a max 50 char commit message headline, without the period.
> >
> > On 29/03/14 19:45, Michał Górny wrote:
> > > +import locale +import math
> > Why not explicitly import what you are going to use? (On the form of
> > "from foo import bar, baz".)
>
> I feel like one of the points in using Python is to explicitly name
> functions (and modules) which you use, rather than importing everything
> in a single namespace and having people scroll up and down to see where
> functions come from.
>
>
I agree with mgorny here. Tons of existing portage code violates this rule,
and it bugs me a lot. I hope that in the future we can move to imports
based on modules (and for things that are really long, explicit assignment,
perhaps to a local.)

-A


> --
> Best regards,
> Michał Górny
>


Re: [gentoo-portage-dev] [PATCH 1/2] Use a localized size formatting function and ISO/IEC prefixes.

2014-03-30 Thread Brian Dolbec
On Sun, 30 Mar 2014 09:56:11 +0200
Michał Górny  wrote:

> Dnia 2014-03-30, o godz. 00:03:02
> Alexander Berntsen  napisał(a):
> 
> > -BEGIN PGP SIGNED MESSAGE-
> > Hash: SHA256
> > 
> > Please use a max 50 char commit message headline, without the
> > period.
> > 
> > On 29/03/14 19:45, Michał Górny wrote:
> > > +import locale +import math
> > Why not explicitly import what you are going to use? (On the form of
> > "from foo import bar, baz".)
> 
> I feel like one of the points in using Python is to explicitly name
> functions (and modules) which you use, rather than importing
> everything in a single namespace and having people scroll up and down
> to see where functions come from.
> 

Yes, I agree for this, it is only used once and is clearer in this
instance.  It might be different if was used many times in the same
file.

-- 
Brian Dolbec 



signature.asc
Description: PGP signature


Re: [gentoo-portage-dev] [PATCH 1/2] Use a localized size formatting function and ISO/IEC prefixes.

2014-03-30 Thread Michał Górny
Dnia 2014-03-30, o godz. 00:03:02
Alexander Berntsen  napisał(a):

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA256
> 
> Please use a max 50 char commit message headline, without the period.
> 
> On 29/03/14 19:45, Michał Górny wrote:
> > +import locale +import math
> Why not explicitly import what you are going to use? (On the form of
> "from foo import bar, baz".)

I feel like one of the points in using Python is to explicitly name
functions (and modules) which you use, rather than importing everything
in a single namespace and having people scroll up and down to see where
functions come from.

-- 
Best regards,
Michał Górny


signature.asc
Description: PGP signature


Re: [gentoo-portage-dev] [PATCH 1/2] Use a localized size formatting function and ISO/IEC prefixes.

2014-03-29 Thread Alexander Berntsen
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Please use a max 50 char commit message headline, without the period.

On 29/03/14 19:45, Michał Górny wrote:
> +import locale +import math
Why not explicitly import what you are going to use? (On the form of
"from foo import bar, baz".)
- -- 
Alexander
berna...@gentoo.org
https://secure.plaimi.net/~alexander
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iF4EAREIAAYFAlM3USUACgkQRtClrXBQc7WGjAD/Sw6gqHnkCKOYlfCcOtdmZGPp
18LK2gWvXZZCDmuVb8QA/1TrnyEb3vsQ/sBe6wyNF0cwmcGQTwjN7b0O2a0CHMUY
=5gkp
-END PGP SIGNATURE-



[gentoo-portage-dev] [PATCH 1/2] Use a localized size formatting function and ISO/IEC prefixes.

2014-03-29 Thread Michał Górny
A similar size formatting function was used in two places in emerge
code. Instead, create a single function in portage.localization module
that formats sizes using the current locale and a common set of rules.

I'm not really convinced about 'ceiling' all sizes but I understand
the original point about not outputting '0 KiB'. If you have better
ideas, I'd be happy to change it.

The code was overall simplified, and a proper number formatting function
is used (instead of string splitting). Locale grouping rules are
respected now, and ISO/IEC binary prefixes are used for unambiguity.
---
 pym/_emerge/resolver/output.py |  5 +++--
 pym/_emerge/resolver/output_helpers.py | 20 +++-
 pym/_emerge/search.py  | 10 +++---
 pym/portage/localization.py| 12 
 4 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/pym/_emerge/resolver/output.py b/pym/_emerge/resolver/output.py
index 5f550be..aefc3f4 100644
--- a/pym/_emerge/resolver/output.py
+++ b/pym/_emerge/resolver/output.py
@@ -18,6 +18,7 @@ from portage.dbapi.dep_expand import dep_expand
 from portage.dep import cpvequal, _repo_separator, _slot_separator
 from portage.eapi import _get_eapi_attrs
 from portage.exception import InvalidDependString, SignatureException
+from portage.localization import localized_size
 from portage.package.ebuild.config import _get_feature_flags
 from portage.package.ebuild._spawn_nofetch import spawn_nofetch
 from portage.output import ( blue, colorize, create_color_func,
@@ -30,7 +31,7 @@ from portage.versions import best, cpv_getversion
 from _emerge.Blocker import Blocker
 from _emerge.create_world_atom import create_world_atom
 from _emerge.resolver.output_helpers import ( _DisplayConfig, _tree_display,
-   _PackageCounters, _create_use_string, _format_size, _calc_changelog, 
PkgInfo)
+   _PackageCounters, _create_use_string, _calc_changelog, PkgInfo)
 from _emerge.show_invalid_depstring_notice import show_invalid_depstring_notice
 
 if sys.hexversion >= 0x300:
@@ -330,7 +331,7 @@ class Display(object):

self.myfetchlist.add(myfetchfile)
if pkg_info.ordered:
self.counters.totalsize += mysize
-   self.verboseadd += _format_size(mysize)
+   self.verboseadd += localized_size(mysize)
 
if self.quiet_repo_display:
# overlay verbose
diff --git a/pym/_emerge/resolver/output_helpers.py 
b/pym/_emerge/resolver/output_helpers.py
index 58b2694..e0ee3e0 100644
--- a/pym/_emerge/resolver/output_helpers.py
+++ b/pym/_emerge/resolver/output_helpers.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2013 Gentoo Foundation
+# Copyright 2010-2014 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 
 """Contains private support functions for the Display class
@@ -17,6 +17,7 @@ import sys
 from portage import os
 from portage import _encodings, _unicode_encode
 from portage._sets.base import InternalPackageSet
+from portage.localization import localized_size
 from portage.output import (blue, bold, colorize, create_color_func,
green, red, teal, turquoise, yellow)
 bad = create_color_func("BAD")
@@ -158,7 +159,7 @@ class _PackageCounters(object):
myoutput.append(", ".join(details))
if total_installs != 0:
myoutput.append(")")
-   myoutput.append(", Size of downloads: %s" % 
_format_size(self.totalsize))
+   myoutput.append(", Size of downloads: %s" % 
localized_size(self.totalsize))
if self.restrict_fetch:
myoutput.append("\nFetch Restriction: %s package" % \
self.restrict_fetch)
@@ -234,21 +235,6 @@ class _DisplayConfig(object):
self.pkg = depgraph._pkg
 
 
-# formats a size given in bytes nicely
-def _format_size(mysize):
-   if isinstance(mysize, basestring):
-   return mysize
-   if 0 != mysize % 1024:
-   # Always round up to the next kB so that it doesn't show 0 kB 
when
-   # some small file still needs to be fetched.
-   mysize += 1024 - mysize % 1024
-   mystr=str(mysize//1024)
-   mycount=len(mystr)
-   while (mycount > 3):
-   mycount-=3
-   mystr=mystr[:mycount]+","+mystr[mycount:]
-   return mystr+" kB"
-
 def _create_use_string(conf, name, cur_iuse, iuse_forced, cur_use,
old_iuse, old_use,
is_new, feature_flags, reinst_flags):
diff --git a/pym/_emerge/search.py b/pym/_emerge/search.py
index bd74fb7..4b0fd9f 100644
--- a/pym/_emerge/search.py
+++ b/pym/_emerge/search.py
@@ -1,4 +1,4 @@
-# Copyright 1999-2013 Gentoo Foundation
+# Copyright 1999-2014 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 
 from __future__