[Piglit] [PATCH v2 1/3] piglit-util-gl: Add piglit_get_gl_enum_from_name

[Ilia Mirkin]

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