Re: [Cython] Cython bug ? - Entry.is_arg flag not set for all arguments

2011-08-14 Thread Vitja Makarov
2011/8/12 Vitja Makarov :
> 2011/8/12 Stefan Behnel :
>> [fixed subject]
>>
>> Romain Guillebert, 12.08.2011 03:19:
>>>
>>> I tried to compiled Demos/primes.pyx using the ctypes backend and I
>>> think I've found a bug :
>>> The Entry for the kmax parameter does not set is_arg to 1. However if I
>>> turn the def into a cdef, it works fine.
>>>
>>> Is this a bug or a known hack ?
>>
>> Vitja already brought this up, too:
>>
>> http://article.gmane.org/gmane.comp.python.cython.devel/12385
>>
>> A quick grep on the sources gives me this:
>>
>> """
>> Cython/Compiler/Buffer.py:            if entry.is_arg:
>> Cython/Compiler/Buffer.py:                if entry.is_arg:
>> Cython/Compiler/ExprNodes.py:        return entry and (entry.is_local or
>> entry.is_arg) and not entry.in_closure
>> Cython/Compiler/FlowControl.py:        return (entry.is_local or
>> entry.is_pyclass_attr or entry.is_arg or
>> Cython/Compiler/FlowControl.py:        self.is_arg = False
>> Cython/Compiler/FlowControl.py:        self.is_arg = True
>> Cython/Compiler/FlowControl.py:                if assmt.is_arg:
>> Cython/Compiler/FlowControl.py:            # TODO: starred args entries are
>> not marked with is_arg flag
>> Cython/Compiler/FlowControl.py:                if assmt.is_arg:
>> Cython/Compiler/FlowControl.py:                    is_arg = True
>> Cython/Compiler/FlowControl.py:                is_arg = False
>> Cython/Compiler/FlowControl.py:            if is_arg:
>> Cython/Compiler/Symtab.py:    # is_arg           boolean    Is the arg of a
>> method
>> Cython/Compiler/Symtab.py:    is_arg = 0
>> Cython/Compiler/Symtab.py:        entry.is_arg = 1
>> """
>>
>> This doesn't look like it would be wrong (or even just unsafe) to consider
>> the unset flag a bug and fix it. Basically, it's almost unused in the
>> original sources, all places where the flag is being read currently are
>> somewhat recent code that looks reasonable to me and that appear to assume
>> that the flag is actually set.
>>
>> So, I'd say, if anyone wants to properly clean this up, please go ahead and
>> do so, but please do it right on the master branch and send it through
>> Jenkins.
>>
>
> Yeah, that would be really nice if all args including starred ones
> will have is_arg attribute set.
>

I've fixed the issue
https://github.com/vitek/cython/commits/_is_arg

It passes all tests if everything is ok I'll push that to upstream

-- 
vitja.
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


[Cython] Problems with decorated methods in cdef classes

2011-08-14 Thread Stefan Behnel

Hi,

I've taken another stab at #593 and changed the way decorators are 
currently evaluated.


http://trac.cython.org/cython_trac/ticket/593

https://github.com/cython/cython/commit/c40ff48f84b5e5841e4e2d2c6dcce3e6494e4c25

We previously had

@deco
def func(): pass

turn into this:

def func(): pass
func = deco(func)

Note how this binds the name more than once and even looks it up in 
between, which is problematic in class scopes and some other special cases. 
For example, this doesn't work:


class Foo(object):
@property
def x(self):
...
@x.setter
def x(self, value):
...

because "x.setter" is looked up *after* binding the second function "x" to 
its method name, thus overwriting the initial property.


The correct way to do it is to create the function object in a temp, pass 
it through the decorator call chain, and then assign whatever result this 
produces to the method name. This works nicely, but it triggered a crash in 
problematic code of another test case, namely "closure_decorators_T478". 
That test does this:


def print_args(func):
def f(*args, **kwds):
print "args", args, "kwds", kwds
return func(*args, **kwds)
return f

cdef class Num:
@print_args
def is_prime(self, bint print_factors=False):
...

Now, the problem is that Cython considers "is_prime" to be a method of a 
cdef class, although it actually is not. It's only an arbitrary function 
that happens to be defined inside of a cdef class body and that happens to 
be *called* by a method, namely "f". It now crashes for me because the 
"self" argument is not being passed into is_prime() as a C method argument 
when called by the wrapper function - and that's correct, because it's not 
a method call but a regular function call at that point.


The correct way to fix this is to turn all decorated methods in cdef 
classes into plain functions. However, this has huge drawbacks, especially 
that the first argument ('self') can no longer be typed as the surrounding 
extension type. But, after all, you could do this:


def swap_args(func):
def f(*args):
return func(*args[::-1])
return f

cdef class Num:
@swap_args
def is_prime(arg, self):
...

I'm not sure what to make of this. Does it make sense to go this route? Or 
does anyone see a way to make this "mostly" work, e.g. by somehow 
restricting cdef classes and their methods? Or should we just add runtime 
checks to prevent bad behaviour of decorators?


Stefan

___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] Cython bug ? - Entry.is_arg flag not set for all arguments

2011-08-14 Thread Stefan Behnel

Vitja Makarov, 14.08.2011 16:57:

Yeah, that would be really nice if all args including starred ones
will have is_arg attribute set.


I've fixed the issue
https://github.com/vitek/cython/commits/_is_arg

It passes all tests if everything is ok I'll push that to upstream


Fine with me.

Stefan
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] Cython bug ? - Entry.is_arg flag not set for all arguments

2011-08-14 Thread Vitja Makarov
2011/8/14 Stefan Behnel :
> Vitja Makarov, 14.08.2011 16:57:
>>>
>>> Yeah, that would be really nice if all args including starred ones
>>> will have is_arg attribute set.
>>
>> I've fixed the issue
>> https://github.com/vitek/cython/commits/_is_arg
>>
>> It passes all tests if everything is ok I'll push that to upstream
>
> Fine with me.
>

Ok, pushed.


-- 
vitja.
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] Problems with decorated methods in cdef classes

2011-08-14 Thread Vitja Makarov
2011/8/14 Stefan Behnel :
> Hi,
>
> I've taken another stab at #593 and changed the way decorators are currently
> evaluated.
>
> http://trac.cython.org/cython_trac/ticket/593
>
> https://github.com/cython/cython/commit/c40ff48f84b5e5841e4e2d2c6dcce3e6494e4c25
>
> We previously had
>
>    @deco
>    def func(): pass
>
> turn into this:
>
>    def func(): pass
>    func = deco(func)
>
> Note how this binds the name more than once and even looks it up in between,
> which is problematic in class scopes and some other special cases. For
> example, this doesn't work:
>
> class Foo(object):
>    @property
>    def x(self):
>        ...
>    @x.setter
>    def x(self, value):
>        ...
>
> because "x.setter" is looked up *after* binding the second function "x" to
> its method name, thus overwriting the initial property.
>
> The correct way to do it is to create the function object in a temp, pass it
> through the decorator call chain, and then assign whatever result this
> produces to the method name.


I'd prefer removal of assignment synthesis from DefNode and
transformation that injects assignment node.
I hope that could make code cleaner.


> This works nicely, but it triggered a crash in
> problematic code of another test case, namely "closure_decorators_T478".
> That test does this:
>
> def print_args(func):
>    def f(*args, **kwds):
>        print "args", args, "kwds", kwds
>        return func(*args, **kwds)
>    return f
>
> cdef class Num:
>    @print_args
>    def is_prime(self, bint print_factors=False):
>        ...
>
> Now, the problem is that Cython considers "is_prime" to be a method of a
> cdef class, although it actually is not. It's only an arbitrary function
> that happens to be defined inside of a cdef class body and that happens to
> be *called* by a method, namely "f". It now crashes for me because the
> "self" argument is not being passed into is_prime() as a C method argument
> when called by the wrapper function - and that's correct, because it's not a
> method call but a regular function call at that point.
>
> The correct way to fix this is to turn all decorated methods in cdef classes
> into plain functions. However, this has huge drawbacks, especially that the
> first argument ('self') can no longer be typed as the surrounding extension
> type. But, after all, you could do this:
>
> def swap_args(func):
>    def f(*args):
>        return func(*args[::-1])
>    return f
>
> cdef class Num:
>    @swap_args
>    def is_prime(arg, self):
>        ...
>
> I'm not sure what to make of this. Does it make sense to go this route? Or
> does anyone see a way to make this "mostly" work, e.g. by somehow
> restricting cdef classes and their methods? Or should we just add runtime
> checks to prevent bad behaviour of decorators?
>

Future CyFunction optimizaition could help here. CyFunction should
accept the following two signatures:

 - self is passed as second (or first?) C-function parameter
 - self is passed inside tuple args

-- 
vitja.
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] Problems with decorated methods in cdef classes

2011-08-14 Thread Stefan Behnel

Vitja Makarov, 14.08.2011 18:55:

2011/8/14 Stefan Behnel:

Hi,

I've taken another stab at #593 and changed the way decorators are currently
evaluated.

http://trac.cython.org/cython_trac/ticket/593

https://github.com/cython/cython/commit/c40ff48f84b5e5841e4e2d2c6dcce3e6494e4c25

We previously had

@deco
def func(): pass

turn into this:

def func(): pass
func = deco(func)

Note how this binds the name more than once and even looks it up in between,
which is problematic in class scopes and some other special cases. For
example, this doesn't work:

class Foo(object):
@property
def x(self):
...
@x.setter
def x(self, value):
...

because "x.setter" is looked up *after* binding the second function "x" to
its method name, thus overwriting the initial property.

The correct way to do it is to create the function object in a temp, pass it
through the decorator call chain, and then assign whatever result this
produces to the method name.


I'd prefer removal of assignment synthesis from DefNode and
transformation that injects assignment node.
I hope that could make code cleaner.


Absolutely.



This works nicely, but it triggered a crash in
problematic code of another test case, namely "closure_decorators_T478".
That test does this:

def print_args(func):
def f(*args, **kwds):
print "args", args, "kwds", kwds
return func(*args, **kwds)
return f

cdef class Num:
@print_args
def is_prime(self, bint print_factors=False):
...

Now, the problem is that Cython considers "is_prime" to be a method of a
cdef class, although it actually is not. It's only an arbitrary function
that happens to be defined inside of a cdef class body and that happens to
be *called* by a method, namely "f". It now crashes for me because the
"self" argument is not being passed into is_prime() as a C method argument
when called by the wrapper function - and that's correct, because it's not a
method call but a regular function call at that point.

The correct way to fix this is to turn all decorated methods in cdef classes
into plain functions. However, this has huge drawbacks, especially that the
first argument ('self') can no longer be typed as the surrounding extension
type. But, after all, you could do this:

def swap_args(func):
def f(*args):
return func(*args[::-1])
return f

cdef class Num:
@swap_args
def is_prime(arg, self):
...

I'm not sure what to make of this. Does it make sense to go this route? Or
does anyone see a way to make this "mostly" work, e.g. by somehow
restricting cdef classes and their methods? Or should we just add runtime
checks to prevent bad behaviour of decorators?


Future CyFunction optimizaition could help here. CyFunction should
accept the following two signatures:

  - self is passed as second (or first?) C-function parameter
  - self is passed inside tuple args


Yes, it would simply take the 'self' argument from the Python arguments, 
check its type and raise an exception if it doesn't match the expected 
type. That's what I meant with "runtime checks". This does sound like a 
reasonable compromise between static extension type semantics and general 
Python semantics. After all, if users want general functions, they can 
always define them outside of the class scope.


There are other issues with decorators in cdef classes, though. For 
example, this


cdef class T:
@staticmethod
@some_deco
def func(arg): pass

is supposed to be different from this:

cdef class T:
@some_deco
@staticmethod
def func(arg): pass

But currently, staticmethod and classmethod change func() directly.

Stefan
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


[Cython] Class scope lookup order

2011-08-14 Thread Vitja Makarov
When creating python-class dict it seems that CPython first looks at
dict then at globals:

A = 1
class X:
A = A

def y():
A = 3
class Y:
A = A
return Y
Y = y()

print(X.A, Y.A)

Will print: 1, 1
And not: 1, 3

I didn't find documentation for this but if I'm correct that should be
easy to fix issue in Cython.

Now for this code cython reports compile-time error: local variable
referenced before assignment


-- 
vitja.
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] Class scope lookup order

2011-08-14 Thread Vitja Makarov
2011/8/15 Vitja Makarov :
> When creating python-class dict it seems that CPython first looks at
> dict then at globals:
>
> A = 1
> class X:
>    A = A
>
> def y():
>    A = 3
>    class Y:
>        A = A
>    return Y
> Y = y()
>
> print(X.A, Y.A)
>
> Will print: 1, 1
> And not: 1, 3
>
> I didn't find documentation for this but if I'm correct that should be
> easy to fix issue in Cython.
>
> Now for this code cython reports compile-time error: local variable
> referenced before assignment
>
>

One more interesting example:

A = 1
def foo(a):
A = a
class X:
a = A
A = A
class Y:
a = A
return X, Y
X, Y = foo(666)
print X.a, X.A, Y.a

-- 
vitja.
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel