Re: [Mesa-dev] [PATCH mesa 2/2] mapi: add asserts to enforce ordering and uniqueness

[Ilia Mirkin]

On Sun, Sep 25, 2016 at 12:27 AM, Eric Engestrom  wrote:
> On Sat, Sep 24, 2016 at 11:39:39PM -0400, Ilia Mirkin wrote:
>> Also note... the list is already sorted. You just picked a different sort
>> order. One that I'm pretty sure is subject to locale settings. As I recall
>> C and utf8 do things differently in a way relevant to this list but I
>> haven't checked.
>
> Oh OK, I didn't think python's sorting would be locale-dependent.
> How would I make it sort using the same method used for the current sort
> order used for this file?

Hrm, well, you could just check it for LC_ALL=C and LC_ALL=en_US.utf8
and hope that it "just works" for the rest. I did check a few corner
cases (like sorting of lower vs upper case, and short vs long strings)
and they do appear to be sorted the same way. This is not what I
remembered - I must have been thinking of some other odd case, perhaps
to do with spaces. Or perhaps Python string comparison does not take
locale into account - that'd be nice.

Yeah, that appears to be the case:

$ LC_ALL=C python -c 'print sorted(["aa", "aA"])'
['aA', 'aa']
$ LC_ALL=en_US.utf8 python -c 'print sorted(["aa", "aA"])'
['aA', 'aa']

But:
$ cat t
aa
aA
$ LC_ALL=en_US.utf8 sort t
aa
aA
$ LC_ALL=C sort t
aA
aa

OK, so the Python2 sort is not locale-dependent. And it does not
appear that they changed this in Python3. Nice!

And actually my comment about sorted returning a generator was wrong
too - it returns a list. That makes sense. Curious to hear whether
Dylan has any comments here.

>
> Note that patch #1 was done using vim's `:sort`, which I do know follows
> the locale sorting order (but I hadn't thought about it until just now).
>
> I guess the best thing is to drop patch #1 and drop the last assert from
> patch #2?  I'll do that tomorrow then.

Hold off on that.

>
> Cheers,
>   Eric
>
>>
>> On Sep 24, 2016 10:27 PM, "Ilia Mirkin"  wrote:
>>
>> > On Sat, Sep 24, 2016 at 10:17 PM, Eric Engestrom 
>> > wrote:
>> > > Dylan Baker recently added functions to that list and had to try a
>> > couple times
>> > > to avoid duplicates. He said [1] he ended up testing it using:
>> > >   len(functions) == len(set(functions))
>> > > which I thought should always be done.
>> > >
>> > > Add this and a couple other tests using asserts to enforce the ordering
>> > and
>> > > uniqueness of the `functions` list, and the uniqueness and compactness
>> > of the
>> > > `offsets` dictionary.
>> > >
>> > > [1] https://lists.freedesktop.org/archives/mesa-dev/2016-
>> > September/129525.html
>> > >
>> > > Signed-off-by: Eric Engestrom 
>> > > ---
>> > >
>> > > If people don't like enforcing the order, I'm happy to drop the previous
>> > patch
>> > > and send a revised version of this patch with this last assert removed.
>> > >
>> > > ---
>> > >  src/mapi/glapi/gen/static_data.py | 6 ++
>> > >  1 file changed, 6 insertions(+)
>> > >
>> > > diff --git a/src/mapi/glapi/gen/static_data.py
>> > b/src/mapi/glapi/gen/static_data.py
>> > > index bb11c1d..ef35b24 100644
>> > > --- a/src/mapi/glapi/gen/static_data.py
>> > > +++ b/src/mapi/glapi/gen/static_data.py
>> > > @@ -435,6 +435,9 @@ offsets = {
>> > >  "MultiTexCoord4sv": 407
>> > >  }
>> > >
>> > > +assert len(offsets) == len(set(offsets.keys())),  "The offsets
>> > dictionary contains duplicates"
>> >
>> > set(offsets) should be enough, I think.
>> >
>> > > +assert len(offsets) == max(offsets.values()) + 1, "The offsets
>> > dictionary has gaps"
>> >
>> > offsets.itervalues()
>> >
>> > > +
>> > >  functions = [
>> > >  "Accum",
>> > >  "ActiveShaderProgram",
>> > > @@ -1723,6 +1726,9 @@ functions = [
>> > >  "WindowPos3svARB",
>> > >  ]
>> > >
>> > > +assert len(functions) == len(set(functions)), "The functions list
>> > contains duplicates"
>> > > +assert functions == sorted(functions),"The functions list is
>> > not sorted"
>> >
>> > I'm surprised this passes. Functions is an array, while sorted() is,
>> > iirc, an iterator (or maybe a generator). Does list.__eq__ have some
>> > sort of special cleverness to deal with that?
>> >
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH mesa 2/2] mapi: add asserts to enforce ordering and uniqueness

[Eric Engestrom]

On Sat, Sep 24, 2016 at 11:39:39PM -0400, Ilia Mirkin wrote:
> Also note... the list is already sorted. You just picked a different sort
> order. One that I'm pretty sure is subject to locale settings. As I recall
> C and utf8 do things differently in a way relevant to this list but I
> haven't checked.

Oh OK, I didn't think python's sorting would be locale-dependent.
How would I make it sort using the same method used for the current sort
order used for this file?

Note that patch #1 was done using vim's `:sort`, which I do know follows
the locale sorting order (but I hadn't thought about it until just now).

I guess the best thing is to drop patch #1 and drop the last assert from
patch #2?  I'll do that tomorrow then.

Cheers,
  Eric

> 
> On Sep 24, 2016 10:27 PM, "Ilia Mirkin"  wrote:
> 
> > On Sat, Sep 24, 2016 at 10:17 PM, Eric Engestrom 
> > wrote:
> > > Dylan Baker recently added functions to that list and had to try a
> > couple times
> > > to avoid duplicates. He said [1] he ended up testing it using:
> > >   len(functions) == len(set(functions))
> > > which I thought should always be done.
> > >
> > > Add this and a couple other tests using asserts to enforce the ordering
> > and
> > > uniqueness of the `functions` list, and the uniqueness and compactness
> > of the
> > > `offsets` dictionary.
> > >
> > > [1] https://lists.freedesktop.org/archives/mesa-dev/2016-
> > September/129525.html
> > >
> > > Signed-off-by: Eric Engestrom 
> > > ---
> > >
> > > If people don't like enforcing the order, I'm happy to drop the previous
> > patch
> > > and send a revised version of this patch with this last assert removed.
> > >
> > > ---
> > >  src/mapi/glapi/gen/static_data.py | 6 ++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/src/mapi/glapi/gen/static_data.py
> > b/src/mapi/glapi/gen/static_data.py
> > > index bb11c1d..ef35b24 100644
> > > --- a/src/mapi/glapi/gen/static_data.py
> > > +++ b/src/mapi/glapi/gen/static_data.py
> > > @@ -435,6 +435,9 @@ offsets = {
> > >  "MultiTexCoord4sv": 407
> > >  }
> > >
> > > +assert len(offsets) == len(set(offsets.keys())),  "The offsets
> > dictionary contains duplicates"
> >
> > set(offsets) should be enough, I think.
> >
> > > +assert len(offsets) == max(offsets.values()) + 1, "The offsets
> > dictionary has gaps"
> >
> > offsets.itervalues()
> >
> > > +
> > >  functions = [
> > >  "Accum",
> > >  "ActiveShaderProgram",
> > > @@ -1723,6 +1726,9 @@ functions = [
> > >  "WindowPos3svARB",
> > >  ]
> > >
> > > +assert len(functions) == len(set(functions)), "The functions list
> > contains duplicates"
> > > +assert functions == sorted(functions),"The functions list is
> > not sorted"
> >
> > I'm surprised this passes. Functions is an array, while sorted() is,
> > iirc, an iterator (or maybe a generator). Does list.__eq__ have some
> > sort of special cleverness to deal with that?
> >
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH mesa 2/2] mapi: add asserts to enforce ordering and uniqueness

[Eric Engestrom]

On Sat, Sep 24, 2016 at 10:27:52PM -0400, Ilia Mirkin wrote:
> On Sat, Sep 24, 2016 at 10:17 PM, Eric Engestrom  wrote:
> > Dylan Baker recently added functions to that list and had to try a couple 
> > times
> > to avoid duplicates. He said [1] he ended up testing it using:
> >   len(functions) == len(set(functions))
> > which I thought should always be done.
> >
> > Add this and a couple other tests using asserts to enforce the ordering and
> > uniqueness of the `functions` list, and the uniqueness and compactness of 
> > the
> > `offsets` dictionary.
> >
> > [1] 
> > https://lists.freedesktop.org/archives/mesa-dev/2016-September/129525.html
> >
> > Signed-off-by: Eric Engestrom 
> > ---
> >
> > If people don't like enforcing the order, I'm happy to drop the previous 
> > patch
> > and send a revised version of this patch with this last assert removed.
> >
> > ---
> >  src/mapi/glapi/gen/static_data.py | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/mapi/glapi/gen/static_data.py 
> > b/src/mapi/glapi/gen/static_data.py
> > index bb11c1d..ef35b24 100644
> > --- a/src/mapi/glapi/gen/static_data.py
> > +++ b/src/mapi/glapi/gen/static_data.py
> > @@ -435,6 +435,9 @@ offsets = {
> >  "MultiTexCoord4sv": 407
> >  }
> >
> > +assert len(offsets) == len(set(offsets.keys())),  "The offsets dictionary 
> > contains duplicates"
> 
> set(offsets) should be enough, I think.

You are right, I'll send a v2 in a minute.

> 
> > +assert len(offsets) == max(offsets.values()) + 1, "The offsets dictionary 
> > has gaps"
> 
> offsets.itervalues()

This doesn't exist in python3 anymore, which means we'd need to explicitly
limit this script to python2. I'm not even sure how to use it, how would that
help?

> 
> > +
> >  functions = [
> >  "Accum",
> >  "ActiveShaderProgram",
> > @@ -1723,6 +1726,9 @@ functions = [
> >  "WindowPos3svARB",
> >  ]
> >
> > +assert len(functions) == len(set(functions)), "The functions list contains 
> > duplicates"
> > +assert functions == sorted(functions),"The functions list is not 
> > sorted"
> 
> I'm surprised this passes. Functions is an array, while sorted() is,
> iirc, an iterator (or maybe a generator). Does list.__eq__ have some
> sort of special cleverness to deal with that?

Honestly, I only dabble with python. I tried this and it worked (on both
python 2 & 3), and I'm guessing underneath it iterates over the two arrays
and compares them element by elements, but that's about as much as I know.

I should've mentioned that my python skill are weak at best, and you are
welcome to suggest improvements :)

Cheers,
  Eric
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH mesa 2/2] mapi: add asserts to enforce ordering and uniqueness

[Ilia Mirkin]

Also note... the list is already sorted. You just picked a different sort
order. One that I'm pretty sure is subject to locale settings. As I recall
C and utf8 do things differently in a way relevant to this list but I
haven't checked.

On Sep 24, 2016 10:27 PM, "Ilia Mirkin"  wrote:

> On Sat, Sep 24, 2016 at 10:17 PM, Eric Engestrom 
> wrote:
> > Dylan Baker recently added functions to that list and had to try a
> couple times
> > to avoid duplicates. He said [1] he ended up testing it using:
> >   len(functions) == len(set(functions))
> > which I thought should always be done.
> >
> > Add this and a couple other tests using asserts to enforce the ordering
> and
> > uniqueness of the `functions` list, and the uniqueness and compactness
> of the
> > `offsets` dictionary.
> >
> > [1] https://lists.freedesktop.org/archives/mesa-dev/2016-
> September/129525.html
> >
> > Signed-off-by: Eric Engestrom 
> > ---
> >
> > If people don't like enforcing the order, I'm happy to drop the previous
> patch
> > and send a revised version of this patch with this last assert removed.
> >
> > ---
> >  src/mapi/glapi/gen/static_data.py | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/mapi/glapi/gen/static_data.py
> b/src/mapi/glapi/gen/static_data.py
> > index bb11c1d..ef35b24 100644
> > --- a/src/mapi/glapi/gen/static_data.py
> > +++ b/src/mapi/glapi/gen/static_data.py
> > @@ -435,6 +435,9 @@ offsets = {
> >  "MultiTexCoord4sv": 407
> >  }
> >
> > +assert len(offsets) == len(set(offsets.keys())),  "The offsets
> dictionary contains duplicates"
>
> set(offsets) should be enough, I think.
>
> > +assert len(offsets) == max(offsets.values()) + 1, "The offsets
> dictionary has gaps"
>
> offsets.itervalues()
>
> > +
> >  functions = [
> >  "Accum",
> >  "ActiveShaderProgram",
> > @@ -1723,6 +1726,9 @@ functions = [
> >  "WindowPos3svARB",
> >  ]
> >
> > +assert len(functions) == len(set(functions)), "The functions list
> contains duplicates"
> > +assert functions == sorted(functions),"The functions list is
> not sorted"
>
> I'm surprised this passes. Functions is an array, while sorted() is,
> iirc, an iterator (or maybe a generator). Does list.__eq__ have some
> sort of special cleverness to deal with that?
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH mesa 2/2] mapi: add asserts to enforce ordering and uniqueness

[Ilia Mirkin]

On Sat, Sep 24, 2016 at 10:17 PM, Eric Engestrom  wrote:
> Dylan Baker recently added functions to that list and had to try a couple 
> times
> to avoid duplicates. He said [1] he ended up testing it using:
>   len(functions) == len(set(functions))
> which I thought should always be done.
>
> Add this and a couple other tests using asserts to enforce the ordering and
> uniqueness of the `functions` list, and the uniqueness and compactness of the
> `offsets` dictionary.
>
> [1] https://lists.freedesktop.org/archives/mesa-dev/2016-September/129525.html
>
> Signed-off-by: Eric Engestrom 
> ---
>
> If people don't like enforcing the order, I'm happy to drop the previous patch
> and send a revised version of this patch with this last assert removed.
>
> ---
>  src/mapi/glapi/gen/static_data.py | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/src/mapi/glapi/gen/static_data.py 
> b/src/mapi/glapi/gen/static_data.py
> index bb11c1d..ef35b24 100644
> --- a/src/mapi/glapi/gen/static_data.py
> +++ b/src/mapi/glapi/gen/static_data.py
> @@ -435,6 +435,9 @@ offsets = {
>  "MultiTexCoord4sv": 407
>  }
>
> +assert len(offsets) == len(set(offsets.keys())),  "The offsets dictionary 
> contains duplicates"

set(offsets) should be enough, I think.

> +assert len(offsets) == max(offsets.values()) + 1, "The offsets dictionary 
> has gaps"

offsets.itervalues()

> +
>  functions = [
>  "Accum",
>  "ActiveShaderProgram",
> @@ -1723,6 +1726,9 @@ functions = [
>  "WindowPos3svARB",
>  ]
>
> +assert len(functions) == len(set(functions)), "The functions list contains 
> duplicates"
> +assert functions == sorted(functions),"The functions list is not 
> sorted"

I'm surprised this passes. Functions is an array, while sorted() is,
iirc, an iterator (or maybe a generator). Does list.__eq__ have some
sort of special cleverness to deal with that?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev