Consider making a new release since deprecation of Python 3.6 and 3.7
marcusvaltonen opened this issue · 9 comments
The deprecation of Python 3.6 and 3.7 (PR #46) and bumping of scipy version to >=1.8.0 is a big step. I would highly recommend doing a release at this point.
For example, I tried building on Python 3.10.2 with scipy==1.8.0 locally (Ubuntu 20.04) with pyproximal==0.2.0, but it failed since the scipy interface changed. Specifically it was line 8 in pyproximal/proximal/VStack.py that failed:
from scipy.sparse.linalg.interface import _get_dtype
Furthermore, when testing my repository in a Docker container (specifically python:3.9.10-slim) I was for this reason forced to use the main branch; however, these standard docker containers do not ship with git installed, and therefore I had to customize the container to first install it before being able to run it.
Maybe not a big deal, but if everything works of the shelf then people are encouraged to use the library more.
Good point, we had the same issue for PyLops and made a release immediately... but did not get around making one for PyProximal yet.
For PyLops as it is a very mature library we decided to handle both pre and post scipy1.8.0 but since this library is fairly new and in early stages I think it is not worth and better to just embrace scipy changes and encourage people not to be left behind in their codes as sooner as later they will need to move beyond python 3.7 and scipy < 1.8.0
I will make this a priority, check what needs changed (thanks for spotting one place to fix) and make a new release next weekend :D This should fix your Docker problem too
Though, I am a bit confused. Are you sure the error was not from pylops.VStack line 8 as there we had what you do changes to handle scipy changes?
Just to clarify: on the current main branch of pyproximal I have not had any issues running Python 3.9 or Python 3.10 with scipy version 1.8.0; this is only on the latest release (0.2.0) where the issue resides. The 0.2.0 release was built without the changes introduced in #46 (which are now on main). The issue was indeed on the line mentioned in the original comment, and as your pylops fix above shows, the correct path to include _get_dtype for scipy version 1.8.0 is
from scipy.sparse.linalg._interface import _get_dtype
and not
from scipy.sparse.linalg.interface import _get_dtype
which is in the 0.2.0 release. Since these lines have been removed afterwards, there is no longer an issue on main.
Right right, my bad! Indeed it was also in Pyproximal and was removed recently.
So we will make a new release which will fix the problem. I think it will be too messy to go back in releases but I will include somewhere in the doc a comment that for <=v0.2.0 to work scipy needs to ee <1.8.0. Do you agree?
As far as I can see in the code, that should be correct. The line
from scipy.sparse.linalg.interface import _get_dtype
has been in the code since version 0.0.0. To be sure, one should probably test it though.
Correct. It was actually there since the start by mistake because it was not needed ;)
I will add this to doc:
You need **Python 3.8 or greater**.
*Note: Versions prior to v0.3.0 work alsi with Python 3.6 or greater, however they
require scipy version to be lower than v1.8.0.*
v0.3.0 is out! This should solve all problems as it forces scipy 1.8.0 to be installed :)
Thank you, I can confirm that it works, and removes my git-dependency in the container. I can now use docker image python:3.9.10-slim directly with no add-ons.