Re: [gentoo-portage-dev] [PATCH gentoolkit 1/2] eclean: Inline _sort_keys method

[Matt Turner]

On Wed, Dec 4, 2019 at 9:17 PM Zac Medico  wrote:
>
> On 12/4/19 6:12 PM, Matt Turner wrote:
> > The boilerplate for calling this method was larger than what it actually
> > contained. Additionally I think this change will allow the loop to run
> > on a generator rather than a full list.
> >
> > Signed-off-by: Matt Turner 
> > ---
> >  pym/gentoolkit/eclean/clean.py | 21 ++---
> >  1 file changed, 6 insertions(+), 15 deletions(-)
> >
> > diff --git a/pym/gentoolkit/eclean/clean.py b/pym/gentoolkit/eclean/clean.py
> > index fd59976..b790de0 100644
> > --- a/pym/gentoolkit/eclean/clean.py
> > +++ b/pym/gentoolkit/eclean/clean.py
> > @@ -36,10 +36,9 @@ class CleanUp(object):
> >   @return: total size that was cleaned
> >   """
> >   file_type = 'file'
> > - clean_keys = self._sort_keys(clean_dict)
> >   clean_size = 0
> > - # clean all entries one by one
> > - for key in clean_keys:
> > + # clean all entries one by one; sorting helps reading
> > + for key in sorted(clean_dict.keys()):
> >   clean_size += self._clean_files(clean_dict[key], key, 
> > file_type)
> >   # return total size of deleted or to delete files
> >   return clean_size
> > @@ -57,10 +56,9 @@ class CleanUp(object):
> >   @return: total size that was cleaned
> >   """
> >   file_type = 'binary package'
> > - clean_keys = self._sort_keys(clean_dict)
> >   clean_size = 0
> > - # clean all entries one by one
> > - for key in clean_keys:
> > + # clean all entries one by one; sorting helps reading
> > + for key in sorted(clean_dict.keys()):
> >   clean_size += self._clean_files(clean_dict[key], key, 
> > file_type)
> >
> >   #  run 'emaint --fix' here
> > @@ -83,10 +81,9 @@ class CleanUp(object):
> >   @return: total size that would be cleaned
> >   """
> >   file_type = 'file'
> > - clean_keys = self._sort_keys(clean_dict)
> >   clean_size = 0
> > - # tally all entries one by one
> > - for key in clean_keys:
> > + # tally all entries one by one; sorting helps reading
> > + for key in sorted(clean_dict.keys()):
> >   key_size = self._get_size(clean_dict[key])
> >   self.controller(key_size, key, clean_dict[key], 
> > file_type)
> >   clean_size += key_size
> > @@ -110,12 +107,6 @@ class CleanUp(object):
> >   print( pp.error("Error: %s" %str(er)), 
> > file=sys.stderr)
> >   return key_size
> >
> > - def _sort_keys(self, clean_dict):
> > - """Returns a list of sorted dictionary keys."""
> > - # sorting helps reading
> > - clean_keys = sorted(clean_dict)
> > - return clean_keys
> > -
> >   def _clean_files(self, files, key, file_type):
> >   """File removal function."""
> >   clean_size = 0
> >
>
> Looks good except you can just use sorted(clean_dict) without calling
> the keys() method.

Thanks Zac! Both fixes you noted (here and in 2/2) are fixed locally.

Re: [gentoo-portage-dev] [PATCH gentoolkit 1/2] eclean: Inline _sort_keys method

[Zac Medico]

On 12/4/19 6:12 PM, Matt Turner wrote:
> The boilerplate for calling this method was larger than what it actually
> contained. Additionally I think this change will allow the loop to run
> on a generator rather than a full list.
> 
> Signed-off-by: Matt Turner 
> ---
>  pym/gentoolkit/eclean/clean.py | 21 ++---
>  1 file changed, 6 insertions(+), 15 deletions(-)
> 
> diff --git a/pym/gentoolkit/eclean/clean.py b/pym/gentoolkit/eclean/clean.py
> index fd59976..b790de0 100644
> --- a/pym/gentoolkit/eclean/clean.py
> +++ b/pym/gentoolkit/eclean/clean.py
> @@ -36,10 +36,9 @@ class CleanUp(object):
>   @return: total size that was cleaned
>   """
>   file_type = 'file'
> - clean_keys = self._sort_keys(clean_dict)
>   clean_size = 0
> - # clean all entries one by one
> - for key in clean_keys:
> + # clean all entries one by one; sorting helps reading
> + for key in sorted(clean_dict.keys()):
>   clean_size += self._clean_files(clean_dict[key], key, 
> file_type)
>   # return total size of deleted or to delete files
>   return clean_size
> @@ -57,10 +56,9 @@ class CleanUp(object):
>   @return: total size that was cleaned
>   """
>   file_type = 'binary package'
> - clean_keys = self._sort_keys(clean_dict)
>   clean_size = 0
> - # clean all entries one by one
> - for key in clean_keys:
> + # clean all entries one by one; sorting helps reading
> + for key in sorted(clean_dict.keys()):
>   clean_size += self._clean_files(clean_dict[key], key, 
> file_type)
>  
>   #  run 'emaint --fix' here
> @@ -83,10 +81,9 @@ class CleanUp(object):
>   @return: total size that would be cleaned
>   """
>   file_type = 'file'
> - clean_keys = self._sort_keys(clean_dict)
>   clean_size = 0
> - # tally all entries one by one
> - for key in clean_keys:
> + # tally all entries one by one; sorting helps reading
> + for key in sorted(clean_dict.keys()):
>   key_size = self._get_size(clean_dict[key])
>   self.controller(key_size, key, clean_dict[key], 
> file_type)
>   clean_size += key_size
> @@ -110,12 +107,6 @@ class CleanUp(object):
>   print( pp.error("Error: %s" %str(er)), 
> file=sys.stderr)
>   return key_size
>  
> - def _sort_keys(self, clean_dict):
> - """Returns a list of sorted dictionary keys."""
> - # sorting helps reading
> - clean_keys = sorted(clean_dict)
> - return clean_keys
> -
>   def _clean_files(self, files, key, file_type):
>   """File removal function."""
>   clean_size = 0
> 

Looks good except you can just use sorted(clean_dict) without calling
the keys() method.
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature

[gentoo-portage-dev] [PATCH gentoolkit 1/2] eclean: Inline _sort_keys method

[Matt Turner]

The boilerplate for calling this method was larger than what it actually
contained. Additionally I think this change will allow the loop to run
on a generator rather than a full list.

Signed-off-by: Matt Turner 
---
 pym/gentoolkit/eclean/clean.py | 21 ++---
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/pym/gentoolkit/eclean/clean.py b/pym/gentoolkit/eclean/clean.py
index fd59976..b790de0 100644
--- a/pym/gentoolkit/eclean/clean.py
+++ b/pym/gentoolkit/eclean/clean.py
@@ -36,10 +36,9 @@ class CleanUp(object):
@return: total size that was cleaned
"""
file_type = 'file'
-   clean_keys = self._sort_keys(clean_dict)
clean_size = 0
-   # clean all entries one by one
-   for key in clean_keys:
+   # clean all entries one by one; sorting helps reading
+   for key in sorted(clean_dict.keys()):
clean_size += self._clean_files(clean_dict[key], key, 
file_type)
# return total size of deleted or to delete files
return clean_size
@@ -57,10 +56,9 @@ class CleanUp(object):
@return: total size that was cleaned
"""
file_type = 'binary package'
-   clean_keys = self._sort_keys(clean_dict)
clean_size = 0
-   # clean all entries one by one
-   for key in clean_keys:
+   # clean all entries one by one; sorting helps reading
+   for key in sorted(clean_dict.keys()):
clean_size += self._clean_files(clean_dict[key], key, 
file_type)
 
#  run 'emaint --fix' here
@@ -83,10 +81,9 @@ class CleanUp(object):
@return: total size that would be cleaned
"""
file_type = 'file'
-   clean_keys = self._sort_keys(clean_dict)
clean_size = 0
-   # tally all entries one by one
-   for key in clean_keys:
+   # tally all entries one by one; sorting helps reading
+   for key in sorted(clean_dict.keys()):
key_size = self._get_size(clean_dict[key])
self.controller(key_size, key, clean_dict[key], 
file_type)
clean_size += key_size
@@ -110,12 +107,6 @@ class CleanUp(object):
print( pp.error("Error: %s" %str(er)), 
file=sys.stderr)
return key_size
 
-   def _sort_keys(self, clean_dict):
-   """Returns a list of sorted dictionary keys."""
-   # sorting helps reading
-   clean_keys = sorted(clean_dict)
-   return clean_keys
-
def _clean_files(self, files, key, file_type):
"""File removal function."""
clean_size = 0
-- 
2.23.0