Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Not exposing class decorator wrapping cdef function to Python #4322

Closed
Bluenix2 opened this issue Aug 1, 2021 · 5 comments
Closed

Comments

@Bluenix2
Copy link
Contributor

Bluenix2 commented Aug 1, 2021

Describe the bug
I am making a convenient extension class wrapping a bitfield, but Cython is not revealing my descriptor class when wrapping a cdef function.

To Reproduce
This has been converted to Pyrex syntax to be explicit (could be reproduced)

cdef class flag:
    """A flag value, works similar to a property by using descriptors."""

    cdef long int mask

    def __init__(self, func) -> None:
        self.mask = func(None)

    def __get__(self, instance, owner):
        """Called when this is accessed through an attribute."""
        if instance is None:
            return self  # Acessed through a class directly

        return (instance.value & self.mask) == self.mask

    def __set__(self, instance, value):
        if value == True:
            instance.value |= self.mask
        else:
            instance.value &= ~self.mask


cdef class Simple:
    """A simple bitfield"""

    cdef int value

    def __init__(self, value):
        self.value = value

    @flag
    cdef one(self):
        return 1 << 1

    @flag
    cdef two(self):
        return 1 << 2

On the latest stable release, this did not compile with the following error:

Error compiling Cython file:
------------------------------------------------------------
...
    cdef int value

    def __init__(self, value):
        self.value = value

   ^
------------------------------------------------------------

flags.pyx:31:4: Cdef functions/classes cannot take arbitrary decorators.

On master branch (installed through pip install -U git+https://github.com/cython/cython) this compiles, but. The issue now becomes that the one and two attributes is not available to Python as demonstrated below:

>>> import flags
>>> s = flags.Simple(1)
>>> dir(s)
['__class__', '__delattr__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__lt__', '__ne__', '__new__', '__pyx_vtable__', '__reduce__', '__reduce_cython__', '__reduce_ex__', '__repr__', '__setattr__', '__setstate__', '__setstate_cython__', '__sizeof__', '__str__', '__subclasshook__']

Expected behavior
I would expect my Simple class to expose 2 instances of my flag extension type called one and two. When I do s.one I would expect it to call __get__. When the flag extension type is initialized, it calls the C function and gets the bit mask.
This is from Python:

class flag:
    """A flag value, works similar to a property by using descriptors."""

    def __init__(self, func) -> None:
        self.mask = func(None)

    def __get__(self, instance, owner):
        """Called when this is accessed through an attribute."""
        if instance is None:
            return self  # Acessed through a class directly

        return (instance.value & self.mask) == self.mask

    def __set__(self, instance, value):
        if value == True:
            instance.value |= self.mask
        else:
            instance.value &= ~self.mask


class Simple:
    """A simple bitfield"""

    def __init__(self, value):
        self.value = value

    @flag
    def one(self):
        return 1 << 1

    @flag
    def two(self):
        return 1 << 2

s = Simple(6)
print(type(Simple.one))  # <class '__main__.flag'>
print(s.one)  # True
print(s.two)  # True

Environment (please complete the following information):

  • OS: Windows
  • Python version 3.9.5
  • Cython version 3.0.0.a9
@da-woods
Copy link
Contributor

da-woods commented Aug 1, 2021

The error is probably pointing to the wrong place, but I would not expect

@flag
 cdef one(self):

to work. I'd expect it to work if declared as a def function I think. But a cdef function is declaring a special function that you can only call directly from Cython, and so it isn't a sensible thing to pass to flag.__init__.

I'm also not sure what it compiles to in the current master (although I doubt that it's sensible...) but cdef functions and attributes generally aren't visible from Python (that's kind of the point of them...) so again I don't think it's a bug.

I think the failure to tell you "Cdef functions/classes cannot take arbitrary decorators." on the current master might be a bug.

@Bluenix2
Copy link
Contributor Author

Bluenix2 commented Aug 1, 2021

but I would not expect

@flag
cdef one(self):

to work.

I disagree, I want to pass the C function to my C extension class's init. I do understand how this would not work if flag came from Python. But flag is C code, it should be able to resolve the pointer and call the function. Inside __get__ it should be able to grab the value from the underlying structure.

I'd expect it to work if declared as a def function I think

When I do this Cython doesn't correctly realize that instance is a C extension class and I will get an attribute error because it tries to look up the attribute "the Python way". This can of course be fixed by setting the underlying value to cdef readonly int value.

but cdef functions and attributes generally aren't visible from Python (that's kind of the point of them...) so again I don't think it's a bug.

My C extension class should be visible to Python though, but I understand how Cython mix it up since the cdef function should not be visible.

I think the failure to tell you "Cdef functions/classes cannot take arbitrary decorators." on the current master might be a bug.

Fair enough, I was originally gonna open a feature request to be able to pass C functions to C extension classes because it's C right.

@da-woods
Copy link
Contributor

da-woods commented Aug 2, 2021

When I do this Cython doesn't correctly realize that instance is a C extension class and I will get an attribute error because it tries to look up the attribute "the Python way". This can of course be fixed by setting the underlying value to cdef readonly int value.

More usefully you have to type instance as Simple and then it'll look up the attribute the Cython way.

I'm also not sure what it compiles to in the current master (although I doubt that it's sensible...)

(replying to myself) It looks like it just drops the decorators and makes them regular cdef methods.

Fair enough, I was originally gonna open a feature request to be able to pass C functions to C extension classes because it's C right.

Everything passed to __init__ must be a Python object, or possible to make from a Python object, because __init__ is part of the Python call mechanism. If you want to construct a C extension class with non-Python inputs then a staticmethod cdef factory function is the usual approach (see last example of https://cython.readthedocs.io/en/latest/src/userguide/extension_types.html#c-methods)


I'll tag this as "error reporting" because Cython is failing to diagnose invalid code correctly. If you need more help getting your use-case to work then the cython-users mailing list is probably the place to follow it up.

@Bluenix2
Copy link
Contributor Author

Bluenix2 commented Aug 2, 2021

If you want to construct a C extension class with non-Python inputs then a staticmethod cdef factory function is the usual approach (see last example of https://cython.readthedocs.io/en/latest/src/userguide/extension_types.html#c-methods)

Ah perfect!

The code now looks like this:

cdef class BaseFlags:
    """The base for all bitfield wrappers."""

    cdef readonly int value

    def __init__(self, value: cython.int) -> None:
        self.value = value

cdef class BitField:
    """A flag value, works similar to a property by using descriptors."""

    cdef int mask

    def __init__(self, mask) -> None:
        self.mask = mask

    def __get__(self, BaseFlags instance, type owner) -> bool:
        if instance is None:
            raise AttributeError(f"type object '{owner}' not initialized.")

        return (instance.value & self.mask) == self.mask

    def __set__(self, BaseFlags instance, bint value) -> None:
        if value == True:
            instance.value |= self.mask
        else:
            instance.value &= ~self.mask

    @staticmethod
    cdef from_func(func):
        return BitField(func(None))


cdef flag(func):
    return BitField.from_func(func)


cdef class Simple(BaseFlags):
    """A simple bitfield"""

    @flag
    def one(self):
        return 1 << 1

    @flag
    def two(self):
        return 1 << 2

This works exactly like expected, unless I change one and two to cdef functions. Trying to access them raises an AttributeError.

da-woods added a commit to da-woods/cython that referenced this issue Aug 2, 2021
These were lost when cdef properties (for extern types) were
introduced.

"Fixes" cython#4322 (based on my interpretation of the problem as an
error-reporting issue)
@scoder
Copy link
Contributor

scoder commented Aug 7, 2021

cdef class BitField:
   …
    @staticmethod
    cdef from_func(func):
        return BitField(func(None))

This looks more like a case for a @classmethod to me. You shouldn't repeat the name of the class here (in case someone wants to subclass it).

This works exactly like expected, unless I change one and two to cdef functions. Trying to access them raises an AttributeError.

cdef functions are (intentionally) not visible to Python code. You can either use cpdef methods, or if you are referring to Cython code using the attributes, then you might have forgotten to type your variable (to let Cython know which cdef attributes there are).

@scoder scoder closed this as completed in ec8c080 Aug 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants