[Cython] Fwd: Re: [cython-users] checking for "None" in nogil function
Dag Sverre Seljebotn
d.s.seljebotn at astro.uio.no
Mon May 7 17:48:14 CEST 2012
On 05/07/2012 03:04 PM, Stefan Behnel wrote:
> Dag Sverre Seljebotn, 07.05.2012 13:48:
>> Here you go:
>> def foo(np.ndarray[double] a, np.ndarray[double] out=None):
>> if out is None:
>> out = np.empty_like(a)
> Ah, right - output arguments. Hadn't thought of those.
> Still, since you pass None explicitly as a default argument, this code
> wouldn't be impacted by disallowing None for buffers by default. That case
> is already handled specially in the compiler. But a better default would
> prevent the *first* argument from being None.
> So, basically, it would do the right thing straight away in your case and
> generate safer and more efficient code for it, whereas now you have to test
> 'a' for being None explicitly and Cython won't understand that hint due to
> insufficient static analysis. At least, since my last commit you can make
> Cython do the same thing by declaring it "not None".
>>>> It's really no different from cdef classes.
>>> I find it at least a bit more surprising because a buffer unpacking
>>> argument is a rather strong hint that you expect something that supports
>>> this protocol. The fact that you type your function argument with it hints
>>> at the intention to properly unpack it on entry. I'm sure there are lots of
>>> users who were or will be surprised when they realise that that doesn't
>>> exclude None values.
>> Whereas I think there would be more users surprised by the opposite.
> We've had enough complaints from users about None being allowed for typed
> arguments already to consider it at least a gotcha of the language.
> The main reason we didn't change this behaviour back then was that it would
> clearly break user code and we thought we could do without that. That's
> different from considering it "right" and "good".
>>>>> And I remember that we wanted to change the default settings for extension
>>>>> type arguments from "or None" to "not None" years ago but never actually
>>>>> did it.
>>>> I remember that there was such a debate, but I certainly don't remember
>>>> that this was the conclusion :-)
>>> Maybe not, yes.
>>>> I didn't agree with that view then and
>>>> I don't now. I don't remember what Robert's view was...
>>>> As far as I can remember (which might be biased towards my personal
>>>> view), the conclusion was that we left the current semantics in place,
>>>> relying on better control flow analysis to make None-checks cheaper, and
>>>> when those are cheap enough, make the nonecheck directive default to
>>> At least for buffer arguments, it silently corrupts data or segfaults in
>>> the current state of affairs, as you pointed out. Not exactly ideal.
>> No different than writing to a field in a cdef class...
> Hmm, aren't those None checked? At least cdef method calls are AFAIR.
Not at all. That's my whole point -- currently, the rule for None in
Cython is "it's your responsibility to never do a native operation on None".
I don't like that either, but that's just inherited from Pyrex (and many
projects would get speed regressions etc.).
I'm not against changing that to "we safely None-check", if done nicely
-- it's just that that should be done everywhere at once.
In current master (and as far back as I can remember), this code:
cdef class A:
cdef int field
cdef int method(self):
cdef A a = None
a.field = 3
__pyx_v_a = ((struct __pyx_obj_5test2_A *)Py_None);
__pyx_v_a->field = 3;
((struct __pyx_vtabstruct_5test2_A *)
> I think we should really get back to the habit of making code safe first
> and fast afterwards.
Nobody has argued otherwise for some time (since the cdivision thread I
believe), this is all about Pyrex legacy. Guess part of the story is
that there's lots of performance-sensitive code in SAGE using cdef
classes which was written in Pyrex before Cython was around...
In fact, the nonecheck directive was written by yours truly! And I
argued for making it the default at the time!
> Because it uses syntax that is expected to unpack the buffer. If that
> buffer doesn't exist, I'd expect an error. It's like using interfaces: I
> want something here that implements the buffer interface. If it doesn't -
> reject it.
> Besides, I hope you are aware that your argumentation stands on the (IMHO
> questionable) fact that "np.ndarray" by itself can be None by default. If
> np.ndarray should not be be allowed to be None by default, why should
> np.ndarray[double]? That argument works in both ways.
I'm well aware of it...
More information about the cython-devel