Support both Cython >3 and Cython < 3

[cmacdonald]

There is some doubts as to whether Cython 3 support works out of the box
#665 (comment) by @misl6

This purpose of /draft/ PR is to demonstrate that the same source as #665 forced to Cython 3 does in fact fail.

jnius/jnius_proxy.pxi:28: in jnius.PythonJavaClass.__init__
    ???
jnius/jnius_proxy.pxi:34: in jnius.PythonJavaClass._init_j_self_ptr
    ???
jnius/jnius_proxy.pxi:162: in jnius.create_proxy_instance
    ???
/usr/local/lib/python3.11/site-packages/jnius/reflect.py:209: in autoclass
    c = find_javaclass(clsname)
jnius/jnius_export_func.pxi:24: in jnius.find_javaclass
    ???
jnius/jnius_export_class.pxi:266: in jnius.JavaClass.__init__
    ???
E   AttributeError: 'Class' object has no attribute '_JavaClass__cls_storage'

Any ideas?

[misl6]

... Yeah.

I trusted make tests too much (and needs to be updated, in order to avoid these issues in future)

• I have 3 virtual envs in kivy/pyjnius (3.9, 3.10, 3.11), and 3.11 was the one used for the test.
• The 3.11 virtual env does not have pytest installed, as pyjnius has been built via pip install -ve . (without the [dev] requirements)
• make test on kivy/kivy (the one I usually work on) calls pytest via python -m pytest and I was expecting the same on kivy/pyjnius. Instead, I have now discovered that on kivy/pyjnius, make tests calls pytest via pytest.
• Unfortunately seems I have pytest installed on my system Python, which is 3.10, and pyjnius is also installed on system Python. (TL;DR: I was running tests on the production pyjnius==1.5.0 🤦)

So, we have an issue with Cython>=3.0.0.

Sorry for the noise. (at least we learned that make tests needs to be fixed, otherwise someone might encounter the same issue I had)

[cmacdonald]

Sorry for the noise.

No problem. If I had responded quicker, we'd had got to this point quicker too.

I think the issue could be:
https://cython.readthedocs.io/en/latest/src/userguide/migrating_to_cy30.html#class-private-name-mangling

jnius/jnius_export_class.pxi:266: in jnius.JavaClass.__init__
    ???
E   AttributeError: 'Class' object has no attribute '_JavaClass__cls_storage'

pyjnius/jnius/jnius_export_class.pxi

Line 266 in 7da4494

cdef JavaClassStorage jcs = self.__cls_storage

So do we need to rewrite all __cls_storage to _JavaClass__cls_storage?

[cmacdonald]

Revised PR, ready for review - This passes the tests locally and on Github.

[misl6]

Sorry for the noise.

No problem. If I had responded quicker, we'd had got to this point quicker too.

I think the issue could be: https://cython.readthedocs.io/en/latest/src/userguide/migrating_to_cy30.html#class-private-name-mangling

jnius/jnius_export_class.pxi:266: in jnius.JavaClass.__init__
    ???
E   AttributeError: 'Class' object has no attribute '_JavaClass__cls_storage'

pyjnius/jnius/jnius_export_class.pxi

Line 266 in 7da4494

cdef JavaClassStorage jcs = self.__cls_storage

So do we need to rewrite all __cls_storage to _JavaClass__cls_storage?

Good catch! However, I guess we should keep compat with both Cython>=3.xx and Cython==0.x.x if it does not take too much resources.

Why?

python-for-android still does not (and will be the next huge change) use a specific Cython version for each recipe. Instead, a default one (0.x.x ATM) is used to cythonize all the recipes.

What do you think about it?

[cmacdonald]

However, I guess we should keep compat with both Cython>=3.xx and Cython==0.x.x if it does not take too much resources.

OK, I had a go at this. I had to add an extra define to config.pxi from setup.py. This is because we needed to:

• detect Cython version at compile time, as Cython isnt available at run time
• be available at "runtime", for the if statement.

The solution isnt particularly elegant, but here we are.

Aside: I note that defs create warnings from Cython 3.0, but this seems to be a rapidly developing situtation (cython/cython#4310) - the latest is that DEFs are OK, but IFs are not?

Should Github actions test on Cython 3 and previous Cython?

[misl6]

The solution isnt particularly elegant, but here we are.

Since it's not here to stay for a quite long time, just looks good.

Aside: I note that defs create warnings from Cython 3.0, but this seems to be a rapidly developing situtation (cython/cython#4310) - the latest is that DEFs are OK, but IFs are not?

Yeah, I've read this discussion multiple times during the past months, and I'm happy that they decided to partially revert their decision on DEF.
IF are fine ATM but we should remove the usage ASAP.

Should Github actions test on Cython 3 and previous Cython?

Would be nice!

[cmacdonald]

Should we update the requirement for the Cython version to leave it verion-agnostic?

requires = [
    "setuptools>=58.0.0",
    "wheel",
    "Cython>=3"
]

I'm not sure how to override the Cython version in the build, as I think the wheels are built in a separate venv?

[cmacdonald]

I'm not sure how to override the Cython version in the build, as I think the wheels are built in a separate venv?

It seems that pyproject.toml (requirements) cannot be overwritten on the commandline, so I used actions in push.yml to force the Cython version by rewriting the pyproject.toml file. Of course, this doubles the number of the GHAs.

I also fixed some of the code for Cython < 3 (so its good we tested this!), and did some general tidying up.

I think this PR is good to go, and hopefully a new shortly release thereafter.

[misl6]

LGTM. Thank you for taking care of it!