On Mon, Feb 23, 2015 at 5:14 PM, Jordan Justen <[email protected]> wrote: > On 2015-02-23 13:55:50, Ilia Mirkin wrote: >> On Mon, Feb 23, 2015 at 4:15 PM, Jordan Justen >> <[email protected]> wrote: >> > On 2015-02-23 10:43:45, Ilia Mirkin wrote: >> >> On Mon, Feb 23, 2015 at 1:23 PM, Jordan Justen >> >> <[email protected]> wrote: >> >> > + @classmethod >> >> > + def get_enums_by_name_in_default_namespace(cls, gl_registry): >> >> > + def append_enum_if_new_value(enum_list, enum): >> >> > + if enum_list[-1].name != enum.name: >> >> > + enum_list.append(enum) >> >> > + return enum_list >> >> > + >> >> > + enums = ( >> >> > + enum >> >> > + for enum_group in gl_registry.enum_groups >> >> > + if enum_group.type == 'default_namespace' >> >> > + for enum in enum_group.enums >> >> > + ) >> >> > + enums = sorted(enums, key=lambda enum: enum.name) >> >> > + enums = reduce(append_enum_if_new_value, enums[1:], [enums[0]]) >> >> > + return enums >> >> >> >> I know you're just copying the other function... >> > >> > Yes, I was trying to be lazy. :) Apparently this code is a little more >> > complex than in needs to be, so I'll try a v3. >> > >> >> what you really want >> >> though is a map from enum name to the enum id. Why not just generate >> >> that and avoid the complication? >> > >> > I don't think a dict is too helpful, since it needs to be sorted and >> > mapped two different ways, and the mapping is not 1:1. >> >> How so? You want 1 name for each id, and you want 1 id for each name. >> Hypothetically you could have multiple id's with the same name and >> vice-versa, which would suck. But even then a (different) dict for >> each would be sufficient. If you iterate over the dict with sorted, >> that'll get you the keys in sorted order... > > Looking at both cases: > 1. Sort on enum, map enum to name, discarding duplicate enums > 2. Sort on name, map name to enum > > Does v3 look reasonable?
First case is more complex than it needs to be, I think. I agree that that's what it does now, but it could just as easily be the map I suggested... (with small template modifications). Instead of % for enum in sorted_unique_enums_in_default_namespace: it could be % for id in sorted(enum_id_to_name): That said, v3 does look correct, if more complex than necessary. I guess I'm mostly reacting to the pattern being used to build up a list avoiding duplicates. While correct, it's rather unpythonic :) And it seems like the dicts would be way simpler and more logical for what's being done here -- creating a mapping. > > Dylan, > > Does v3 look better regarding your feedback? (Ie, lose the list > comprehensions...) > > -Jordan _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
