Re: [gentoo-releng] Enable USE_EXPAND support in catalyst

[Anthony G. Basile]

On 02/15/15 02:54, Rémi Cardona wrote:
> Le 14/02/2015 20:03, Anthony G. Basile a écrit :
> In the last chunk of modules/generic_stage_target.py:
>
> * type(var) == types.DictType can be replaced with
>    isinstance(var, dict).
>
>    Same thing for StringType/str, ListType/list and BooleanType/bool.
>    This is how simple types were checked before Python 2.2 (around 2002).
>
> * string.replace has been deprecated for ages, use str.replace directly:
>
>    varname2="clst_"+string.replace(y,"/","_")
>    varname2=string.replace(varname2,"-","_")
>    varname2=string.replace(varname2,".","_")
>
>    becomes
>
>    varname2 = "clst_" + y.replace("/", "_")
>    varname2 = varname2.replace("-", "_")
>    varname2 = varname2.replace(".", "_")
>
> * string.join is also deprecated, use ' '.join() instead
>
> * dict objects are iterable (and they return keys). This means that
>
>    string.join(self.settings[x].keys())
>
>    can be written (with the join suggestion above):
>
>    ' '.join(self.settings[x])

Looking at that code was like a walk down memory lane.  Those idioms are 
used consistently in many different places throughout the code. I'd 
rather not break from those idioms in this commit because then you get 
mixed code.  If we want to modernize the codebase then we should just 
grep the source tree and do massive commits switching string.replace() 
with <str obj>.replace() etc.

So I'm not disagreeing with you here, but not for this commit.

>
> * You may want to use dict.items() to get both the key and the
>    associated value:
>
>    for y in self.settings[x].keys():
>
>    becomes:
>
>    for y, val in self.settings[x].items():
>
>    which should allow you to replace "self.settings[x][y]" with "val"
>
> Cheers,
>
> Rémi
>

I don't know when d.items() came to python but this one bothers me 
enough that I'll switch --- I never remember it not being there. There 
are a few other places using this idiom in the source, but at least not 
in generic_stage_target.py.  Maybe I'll submit a few cleanup commits, 
but with catalyst 3.x coming, its hard to justify putting the effort 
into it.  There's some pretty scary insanity in 2.x like take a look at 
line 662 of modules/generic_stage_target.py.

@Brian.  What do you think?  Should I spend some time modernizing 2.x or 
just wait for 3.x?

-- 
Anthony G. Basile, Ph.D.
Gentoo Linux Developer [Hardened]
E-Mail    : blueness@g.o
GnuPG FP  : 1FED FAD9 D82C 52A5 3BAB  DC79 9384 FA6E F52D 4BBA
GnuPG ID  : F52D4BBA