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] cpdef module-level variables not exposed to Python #3959

Closed
leofang opened this issue Dec 28, 2020 · 6 comments
Closed

[BUG] cpdef module-level variables not exposed to Python #3959

leofang opened this issue Dec 28, 2020 · 6 comments

Comments

@leofang
Copy link

leofang commented Dec 28, 2020

Describe the bug

For a cpdef variable declared at the module level like this

# test_cpdef_module_var.pyx

...
cpdef str abc = 'this is abc'
...

I would expect that I can access this Python string in the Python space simply by test_cpdef_module_var.abc. However, this appears to be not possible, and we had to work around it by explicitly declaring a Python object referencing it:

# test_cpdef_module_var.pyx

...
cpdef str _abc = 'this is abc'
abc = _abc
...

which is both counterintuitive and cumbersome. I tried searching in the issue tracker but it doesn't seem to be reported yet, and the closest report I found is this (#656) but not exactly the same, so I hope I am simply missing something obvious...

To Reproduce
See above.

Expected behavior
See above.

Environment (please complete the following information):

  • OS: Linux
  • Python version: 3.7.8
  • Cython version: 0.29.21
@da-woods
Copy link
Contributor

da-woods commented Dec 28, 2020

@leofang cpdef module level variables don't actually exist (hence they don't work) - in most versions of Python it isn't possible to implement module level properties so there's simply not way for expose a C variable for Python. Obviously for your str it is ultimately a Python object, but that's the general rule on why it doesn't work.

In more recent versions it might be possible to do using a module-level __getattr__ but it definitely isn't implemented yet.

Cython should probably tell you that they don't work, so this bug is really a failure of error reporting.

da-woods added a commit to da-woods/cython that referenced this issue Jan 1, 2021
Allowing these gives people the false impression that they
do something meaningful. "Fixes" cython#3959
@leofang
Copy link
Author

leofang commented Jan 2, 2021

Hi @da-woods Thanks for quick reply during the holiday season!

@leofang cpdef module level variables don't actually exist (hence they don't work) - in most versions of Python it isn't possible to implement module level properties so there's simply not way for expose a C variable for Python. Obviously for your str it is ultimately a Python object, but that's the general rule on why it doesn't work.

Interesting, though I am more confused. I thought that module-level cdef variables can always be cimported, and that module-level Python objects can always be imported. For example, this works fine:

$ cat test_cython_cpdef1.pxd
cdef str abc

$ cat test_cython_cpdef1.pyx
cdef str abc = 'xyz'

$ cat test_cython_cpdef2.pyx
from test_cython_cpdef1 cimport abc

cpdef print_str():
    print(abc)  # so calling test_cython_cpdef2.print_str() in Python will output "xyz"

So what's special with cpdef variables? Can't a cpdef variable be split into two objects? One lives in C and the other stays in the module's dict as a Python object (like abc above in the line abc = _abc), and then refers to its C counterpart when accessed?

In more recent versions it might be possible to do using a module-level __getattr__ but it definitely isn't implemented yet.

By recent version do you mean Python 3.9+?

Cython should probably tell you that they don't work, so this bug is really a failure of error reporting.

So I suppose there're two failures for this bug:

  1. Poor error reporting for older versions (of whatever you referred to)
  2. Missing implementation for handling it properly in newer versions (of whatever)

Am I right?

@leofang
Copy link
Author

leofang commented Jan 2, 2021

I just saw #3963 and as mentioned there a warning should be raised, not an error.

@da-woods
Copy link
Contributor

da-woods commented Jan 2, 2021

Interesting, though I am more confused. I thought that module-level cdef variables can always be cimported, and that module-level Python objects can always be imported.

Right - I think:

  1. Module-level cdef variables can always be cimported
  2. Module-level variables defined in the normal Python way (abc = "something", no cdef) can always be imported, and are Python objects.
  3. It'd be fairly difficult to make a variable where internal type is a C type (e.g. cdef int x), but is exposed to Python. That is what public and readonly module globals #656 is about. There's currently no way of doing this.
  4. It'd be surprisingly difficult to do what you want, where the internal type is a Python object and the variable is exposed to Python. It's very difficult to stop Python users reassigning the version they have access to for example. Because of that module-level cdef object variables are treated the same way as any other module-level cdef variable and hidden from Python.

Using PEP 526 would allow readonly variables on Python 3.7+ (so probably isn't a complete solution)


One lives in C and the other stays in the module's dict as a Python object (like abc above in the line abc = _abc), and then refers to its C counterpart when accessed?

This has the problem that the Python user can do module.abc = 10 and suddenly the variables are out of sync.


So I suppose there're two failures for this bug:

  1. Poor error reporting for older versions (of whatever you referred to)
  2. Missing implementation for handling it properly in newer versions (of whatever)

Am I right?

Point 2 is essentially #656 (although possibly with different syntax - it's suggesting using public and readonly instead of cpdef). But essentially yes.

@leofang
Copy link
Author

leofang commented Jan 3, 2021

Thanks, @da-woods. I understand now what you meant by out of sync. This is certainly unexpected to me:

# test_cpdef_module_var.pyx
cpdef str _abc = 'this is abc'

abc = _abc

cpdef get_python_var():
    print(abc)

cpdef set_cython_var(a):
    global _abc
    _abc = a

cpdef get_cython_var():
    print(_abc)
>>> import test_cpdef_module_var
>>> test_cpdef_module_var.abc
'this is abc'
>>> test_cpdef_module_var.get_python_var()
this is abc
>>> test_cpdef_module_var.get_cython_var()
this is abc
>>> test_cpdef_module_var.set_cython_var('xyz')
>>> test_cpdef_module_var.get_cython_var()
xyz
>>> test_cpdef_module_var.get_python_var()  # <-- out of sync!
this is abc

Though I think for my purposes having cpdef variables exposed to Python as readonly would be more than enough 🙂

@TeamSpen210
Copy link
Contributor

A module just stores the attributes in a simple dict, so there's no real control or hooks for modifying what happens when names are accessed. As of Python 3.7 and PEP 562 a __getattr__ function will be called which would allow readonly variables, but that could still be bypassed by a user setting the attribute.

Cython would really need to produce a ModuleType subclass. Multi-phase initialisation supports Py_mod_create and single-phase just returns the module, so it seems possible to do (but quite complicated).

da-woods added a commit to da-woods/cython that referenced this issue Jan 8, 2021
Allowing these gives people the false impression that they
do something meaningful. "Fixes" cython#3959
@scoder scoder closed this as completed in 875584d Jul 2, 2021
da-woods added a commit to da-woods/cython that referenced this issue Jul 11, 2021
Allowing these gives people the false impression that they do something meaningful.

Closes cython#887
Closes cython#3959
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