Rerun with cython 0.29.35 to fix Python 3.11 compatibility issues

[jeremysanders]

This should fix the Python 3.11 compatibility problems by rerunning cython using 0.29.35.

See bugs:
#1018
#998

[codecov]

Codecov Report

Merging #1020 (e91799a) into devel (9ec3e50) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##            devel    #1020   +/-   ##
=======================================
  Coverage   54.49%   54.49%           
=======================================
  Files         210      210           
  Lines       21560    21560           
  Branches     3169     3169           
=======================================
  Hits        11750    11750           
  Misses       9255     9255           
  Partials      555      555           

[bpkroth]

@zhenwendai @MartinBubel, you're some of the last commit authors in the history. Any chance we can get this committed and a new version cut soon? There are several projects that rely on it that are currently having issues with recently Python and numpy version mismatches.

[MartinBubel]

Thanks for picking this up @jeremysanders . @bpkroth I don't have the rights to merge and even though being among the recent contributors, my experience with GPy is very limited.

However, I spend some time with this and did the following:

1. Clone the pull request
2. Run a python3.11-slim-buster container
3. Install python3-dev and build-essential
4. Run python setup.py develop
5. Run tests

Running pytest, I get one failed test with the following error message:

root@ad1ad64679d9:/GPy# pytest .
================================================================== test session starts ==================================================================
platform linux -- Python 3.11.4, pytest-7.4.0, pluggy-1.2.0
rootdir: /GPy
collected 0 items / 1 error                                                                                                                             

======================================================================== ERRORS =========================================================================
______________________________________________________ ERROR collecting GPy/testing/linalg_test.py ______________________________________________________
ImportError while importing test module '/GPy/GPy/testing/linalg_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/local/lib/python3.11/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
GPy/__init__.py:32: in <module>
    from numpy.testing import Tester
E   ImportError: cannot import name 'Tester' from 'numpy.testing' (/usr/local/lib/python3.11/site-packages/numpy/testing/__init__.py)
-------------------------------------------------------------------- Captured stdout --------------------------------------------------------------------
warning in stationary: failed to import cython module: falling back to numpy
warning in coregionalize: failed to import cython module: falling back to numpy
warning in choleskies: failed to import cython module: falling back to numpy
================================================================ short test summary info ================================================================
ERROR GPy/testing/linalg_test.py
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
=================================================================== 1 error in 1.55s ====================================================================

I could not immediately figure out what the actual cause of the error is but I would not surprised if this is another "old thing" causing problems nowadays.

@jeremysanders what was your approach on testing?

Anyways "lgtm". But I think it would be better if one the maintainers can take a look at this.

[bpkroth]

I took a brief look at this:

E ImportError: cannot import name 'Tester' from 'numpy.testing'

That's basically a shim that numpy was providing for running tests with "nose", which is what GPy still uses.

nose is also very out of date and not maintained. I imagine some hacks could be done to support it, but it's more than I have time to dig into atm.

[MartinBubel]

I can take a look at the tests, but not guarantee that I will get it done in the next couple of days...

But imho it would be best to consider this an opportunity and get rid of outdated / deprecated testing envionrments, so my intended fix would be to update the tests (if possible). Doest that sound good for you @bpkroth ?

[MartinBubel]

Also, I wonder whether we should start a new PR for adding some CI job that builds the cython files for each deploy/release.

I don't want to sound too ambitious on this but keeping cython-files in the repo might not be the best idea (as we have seen in the issues leading to this PR).

[MartinBubel]

I propse that we merge this PR as this point. As mentioned above, I don't like the fact to much that the cython files are in the repo but that seems to be the case for GPy at the moment. And since no github-actions are failing, there is nothing holding us back from merging this.

Regarding the test and deploy changes proposed above, I recommend that we start a separate PR to account for them.

[haampie]

Alternatively, remove the generated C sources. That's what cython people advise now, since they cannot guarantee forward compatibility with python.

[ekalosak]

Ok, so this one is indeed a bigger fish than the contributor is frying. Thanks very much everyone for bringing this up, and to the contributor for providing an interim fix.

@lawrennd what are your thoughts on the urgency and strategy for (a) removing the generated C (b) digging out of the nose tech tebt and (c) merging this PR to support 3.11 in the meantime?

Unless you've got strong feelings on (c) I think it's better to make the pragmatic decision to support 3.11 as soon as possible. For (a) and (b), I suspect there may be less-than-obvious ramifications.

[ekalosak]

My one concern with dropping the generated C code is that it may make pip install GPy take ages.
@haampie can you link to more info on the guidance for generated C? This could be sticky, best to be sure before we just drop it IMHO.

[lawrennd]

I'm less familiar with the technical challenges, but I agree with your analysis. It feels like (c) is a priority, and (a) and (b) are also important but may require some thought. I feel a bit too distant from the technical details to be doing the merge, but I've just sent @MartinBubel an invite on that front.

[haampie]

@ekalosak the discussion here: cython/cython#5089 (comment)

[ekalosak]

Ok so it seems like the right path forward is to (a) merge this immediately (b) open a discussion issue for removing the compiled cython from the repo.

I'll move my concerns about degrading user build times to that discussion thread.