[Piglit] [PATCH v2 1/3] piglit-util-gl: Add piglit_get_gl_enum_from_name
imirkin at alum.mit.edu
Mon Feb 23 14:26:11 PST 2015
On Mon, Feb 23, 2015 at 5:14 PM, Jordan Justen
<jordan.l.justen at intel.com> wrote:
> On 2015-02-23 13:55:50, Ilia Mirkin wrote:
>> On Mon, Feb 23, 2015 at 4:15 PM, Jordan Justen
>> <jordan.l.justen at intel.com> wrote:
>> > On 2015-02-23 10:43:45, Ilia Mirkin wrote:
>> >> On Mon, Feb 23, 2015 at 1:23 PM, Jordan Justen
>> >> <jordan.l.justen at intel.com> 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])
>> >> > + 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).
% 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.
> Does v3 look better regarding your feedback? (Ie, lose the list
More information about the Piglit