[Cython] remaining open issues for 0.17

[Stefan Behnel]

mark florisson, 09.08.2012 18:51:
> On 9 August 2012 16:36, Stefan Behnel wrote:
>> Stefan Behnel, 09.08.2012 14:31:
>>> mark florisson, 07.08.2012 11:09:
>>>> I thought the 32 bit issue was resolved? You pushed a fix and I fixed
>>>> some tests, so it passed for me. I can run it again to check...
>>>
>>> I don't know. Yaroslav replied to your mail saying that your fixes didn't
>>> change anything for the Debian builds. Let's see what he has to say about
>>> the second beta.
>>
>> He ran them through the build servers. Here are the latest results:
>>
>> https://buildd.debian.org/status/package.php?p=cython&suite=experimental
>>
>> (hint: click on "all" in the Logs column, then click on the build result to
>> see the log output)
>>
>> The i386 tests show 4 errors related to fused types in the NumPy tests:
>>
>> https://buildd.debian.org/status/fetch.php?pkg=cython&arch=i386&ver=0.17~beta1-2&stamp=1343428255
>>
>> >From a quick look, they might really be problems in the tests rather than
>> the code we generate.
>>
>> It also looks like my signed char fix was not sufficient, as you can see in
>> the memslice tests here, e.g. in the
>> "memslice.__test__.test_memslice_struct_with_arrays" test:
>>
>> https://buildd.debian.org/status/fetch.php?pkg=cython&arch=s390&ver=0.17~beta1-2&stamp=1343418358
>>
>> It now prints
>>
>> """
>> ValueError: Buffer dtype mismatch, expected 'char' but got 'unsigned char'
>> in 'ArrayStruct.chars'
>> """
>>
>> Well, "char" and "unsigned char" are supposed to be the same on this
>> platform, but I guess it can't know that. I wonder if "char" and "signed
>> char" are considered identical on other platforms ...
>>
>> To fix the tests, it might be enough to use an explicit "unsigned char"
>> instead of plain "char" for the types. Not sure if that's a good idea or if
>> there actually is a bug hidden below this test failure. At least, it smells
>> like an inconvenience for users to get that error.
> 
> I think these tests are fine (unlike the fused test I fixed), it's a
> bug somewhere. I'm not sure where though, because the code that
> creates the format string uses the type info struct (with the C
> compile-time unsigned check), and so does the buffer format parser.
> The code creating the type strings are in the MemoryView.pyx and
> Buffer.c utility files at the bottom. It does
> 
> if (type->is_unsigned)
>     *buf = toupper(*buf);
> 
> So whatever I feed it on my system (char, signed char or unsigned
> char), it generates the right format string for me.

I think this needs some more investigation directly on the failing systems.
It'll be much easier to figure this out with a debugger.


> Perhaps we can just ignore these test failures, I don't think many
> people are using memoryviews with characters. Changing them to signed
> or unsigned char may fix the problem for the tests, though.

If we can't find a way to fix this in time, I'm leaning towards that, too.
However, working around a bug in a test is a sure way to forget about the
bug and consider everything working. Could you copy the failing tests into
a separate test case, disable that in bugs.txt and then change the types in
the original test modules to whatever you think would make the tests work?


> Is it bad to release something that doesn't pass the entire test suite
> on some system?

Given that we already made tons of releases without even knowing that
they'd fail on some systems, I'd say no. :)

So far, we've always taken care of the most common systems and just tried
to accommodate for other systems to a certain extent. It really doesn't
matter if we fix these things before or after the 0.17 release.

Stefan