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

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

[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 >= 0x3000000:
@@ -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__ import print_function
@@ -7,6 +7,7 @@ import re
 import portage
 from portage import os
 from portage.dbapi.porttree import _parse_uri_map
+from portage.localization import localized_size
 from portage.output import  bold, bold as white, darkgreen, green, red
 from portage.util import writemsg_stdout
 
@@ -348,12 +349,7 @@ class search(object):
 							break
 
 					if myebuild and file_size_str is None:
-						mystr = str(mysum[0] // 1024)
-						mycount = len(mystr)
-						while (mycount > 3):
-							mycount -= 3
-							mystr = mystr[:mycount] + "," + mystr[mycount:]
-						file_size_str = mystr + " kB"
+						file_size_str = localized_size(mysum[0])
 
 					if self.verbose:
 						if available:
diff --git a/pym/portage/localization.py b/pym/portage/localization.py
index b54835a..e4d87b6 100644
--- a/pym/portage/localization.py
+++ b/pym/portage/localization.py
@@ -2,6 +2,9 @@
 # Copyright 2004-2014 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 
+import locale
+import math
+
 from portage import _unicode_decode
 
 # We define this to make the transition easier for us.
@@ -25,3 +28,12 @@ def localization_example():
 	print(_("A: %(a)s -- B: %(b)s -- C: %(c)s") %
 	      {"a": a_value, "b": b_value, "c": c_value})
 
+def localized_size(num_bytes):
+	"""
+	Return pretty localized size string for num_bytes size
+	(given in bytes). The output will be in kibibytes.
+	"""
+
+	# always round up, so that small files don't end up as '0 KiB'
+	num_kib = math.ceil(num_bytes / 1024)
+	return locale.format('%d', num_kib, grouping=True) + ' KiB'
-- 
1.9.1

[gentoo-portage-dev] [PATCH 2/2] emerge: call setlocale() to enable system locale.

[Michał Górny]

This is necessary so that the size formatting function (and possibly
other locale-dependant functions in the future) respect the system
locale rather than using the 'C' locale.
---
 pym/_emerge/main.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/pym/_emerge/main.py b/pym/_emerge/main.py
index cfe1332..eddb16c 100644
--- a/pym/_emerge/main.py
+++ b/pym/_emerge/main.py
@@ -3,6 +3,7 @@
 
 from __future__ import print_function
 
+import locale
 import platform
 import sys
 
@@ -984,6 +985,9 @@ def emerge_main(args=None):
 
 	args = portage._decode_argv(args)
 
+	# Use system locale.
+	locale.setlocale(locale.LC_ALL, '')
+
 	# Disable color until we're sure that it should be enabled (after
 	# EMERGE_DEFAULT_OPTS has been parsed).
 	portage.output.havecolor = 0
-- 
1.9.1

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

[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
bernalex@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-----

Re: [gentoo-portage-dev] [PATCH 2/2] emerge: call setlocale() to enable system locale.

[Alexander Berntsen]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Please remove the period from the commit message. I would prefer if
you used an explicit import here as well.

Other than those minor complaints, these patches look good to me.
- -- 
Alexander
bernalex@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/

iF4EAREIAAYFAlM3UXEACgkQRtClrXBQc7V37QEAqqQxVz4TTCw2lrWOy8+rIJt4
uq1ygRb+4p3ohABkSa8A/1pkqg571xpC87mHDUT88957aUJHpWY5tzbB6GB1dHMW
=TL14
-----END PGP SIGNATURE-----

Re: [gentoo-portage-dev] [PATCH 2/2] emerge: call setlocale() to enable system locale.

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 367 bytes --]

Dnia 2014-03-30, o godz. 00:04:17
Alexander Berntsen <bernalex@gentoo.org> napisał(a):

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> Please remove the period from the commit message. I would prefer if
> you used an explicit import here as well.

Explicit as in:

  from locale import setlocale, LC_ALL

?

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]

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

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]

Dnia 2014-03-30, o godz. 00:03:02
Alexander Berntsen <bernalex@gentoo.org> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]

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

[Brian Dolbec]

[-- Attachment #1: Type: text/plain, Size: 944 bytes --]

On Sun, 30 Mar 2014 09:56:11 +0200
Michał Górny <mgorny@gentoo.org> wrote:

> Dnia 2014-03-30, o godz. 00:03:02
> Alexander Berntsen <bernalex@gentoo.org> 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 <dolsen>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

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

[Alec Warner]

[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]

On Sun, Mar 30, 2014 at 12:56 AM, Michał Górny <mgorny@gentoo.org> wrote:

> Dnia 2014-03-30, o godz. 00:03:02
> Alexander Berntsen <bernalex@gentoo.org> 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
>

[-- Attachment #2: Type: text/html, Size: 1783 bytes --]