search

populse / soma-base

Miscelaneous all-purpose classes and functions
0 stars 3 forks source link

Updated advanced customization of QT module import process #84

Closed servoz closed 4 months ago

servoz commented 4 months ago

As discussed in private messages, the current advanced customization of the soma QT module import process is not working since python 3.12.

In the qt_backend module, finders and loaders have been outdated for several versions of Python.

Since python 3.4, the find_spec() method of the finders metapath has replaced find_module(), which is now obsolete.

Between python 3.4 and python 3.11, the import machinery still accepts find_module(), but since python 3.12 find_module() has been removed.

As for the loader, since python 3.4, the load_module() method has been replaced by exec_module(). In addition, the create_module() method is no longer optional when exec_module() is defined. Since Python 3.4, the recommended API for loading a module is exec_module() (and create_module()).

It seems to me that the substance of what has been done is still good, and that this PR is above all an update of the machinery in line with current recommendations (since Python 3.4).

I've just tested with python 3.9 and the changes haven't introduced any regression.

I won't have access to python 3.10 and especially 3.12 until next Friday. I'll be testing with these two versions of python and I think that if there are no regressions we'll be able to merge this branch.

I'll be back here on Friday to give the result with python 3.12 (and 3.10).

I also noticed that the crash in UT for the master branch are also in the qt_backend (I haven't looked into why, but I can have a look if it's useful...).

sapetnioc commented 4 months ago

These changes must be done on branch 6.0 too. I am not sure what is the best option for that with git (I believe master branch is too far from 6.0 to be merged). I will create another PR based on 6.0 branch and cherry-picking this commit.

denisri commented 4 months ago

The changes do work on python 3.10, so they are not breaking anything. So I guess we should merge.

These changes must be done on branch 6.0 too.

Yes, I think the best option is to cherry-pick later on 6.0. Merging has become impossible I think.

denisri commented 4 months ago

I also noticed that the crash in UT for the master branch are also in the qt_backend (I haven't looked into why, but I can have a look if it's useful...).

This is because UTs in appveyor are running python 2.7 and 3.5, and the code includes f-strings, which appeared in python 3.6. As soma-base 6.0 will require at least python 3.9, I guess we should drop support of python < 3.9.

denisri commented 4 months ago

Well I'm not familiar with appveyor and I have failed to switch to other python versions (tests are still running 2.7) in 822cd15efbde74be46826b40102650d56cbb639e

sapetnioc commented 4 months ago

I changed AppVeyor file on branch python3.12 (i.e. changed the PR file)

servoz commented 4 months ago

I also think that since the change doesn't cause any regression with python 3.9 and 3.10 there shouldn't be any problem with 3.12 (since these changes were made mainly for python 3.12 !) but I'd like to wait to test with python 3.12 to be sure that everything is fine (I'll have access to the python 3.12 version on Friday, I'll confirm then that everything is fine).

I've made sure that appveyor works correctly with python 3.9 and python 3.11.

I will come back here friday ...

denisri commented 4 months ago

Thanks @servoz for the appveyor fix d554ad7cedb04e17659ceadd97234a7ed0365e5c I have back-ported it to master also :)

servoz commented 4 months ago

I had to make a small modification and everything now seems to work correctly with python 3.12. OK to merge?

denisri commented 4 months ago

Great ! Let's merge...