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

Python 3.8 compatibility broken #773

Closed
okahilak opened this issue May 17, 2024 · 12 comments
Closed

Python 3.8 compatibility broken #773

okahilak opened this issue May 17, 2024 · 12 comments

Comments

@okahilak
Copy link
Collaborator

okahilak commented May 17, 2024

After #768 was merged, Python 3.8 compatibility seems to be broked. When starting InVesalius using Python 3.8, it gives the error:

File "C:\users\mtms\invesalius3\invesalius\presets.py", line 79, in Presets
    def UpdateThresholdModes(self, threshold_range: tuple[int, int]) -> None:
TypeError: 'type' object is not subscriptable

For Python versions older than 3.9, the type hint for tuple would need to be separately imported before it can be used:

from typing import Tuple
@tfmoraes
Copy link
Member

@okahilak, @paulojamorim, @rmatsuda and @vhosouza we need to choose the minimum python version we are support. Is it necessary to be 3.8? Can we use a newer version? I'm using 3.11 and 3.12.

@paulojamorim
Copy link
Collaborator

Hi @tfmoraes

I am using Ubuntu 20.04 with Python 3.8. The problem is that wxPython and VTK from pip are causing issues on my system when rendering a wxVTKRenderWindow. This is happening with both Python 3.8 and 3.12 (manually installed on Ubuntu 20.04).

This is the error after InVesalius closes.

2024-05-19 10:31:46.264 ( 35.614s) [ 7F4D5E640280]vtkOpenGLRenderWindow.c:704 ERR| vtkXOpenGLRenderWindow (0x55979e061650): GLEW could not be initialized: Missing GL version

@omar-abdelgawad
Copy link
Contributor

Hi @tfmoraes
I mentioned the importance of choosing the minimum python in this discussion before making a PR. I should have mentioned the backwards compatibility issue with python 3.8. The default python version for ubuntu 22.04 is 3.10 which is the one I am using.

Regarding this topic, I believe this Status of Python versions table might be helpful in choosing the minimum version to support. I want to also mention that 3.8 is the current stable version with about 5 months remaining where 3.7 has reached its 'end of life' about 7 months ago.

Regarding typing, A lot of features are present in the later versions after 3.8 so if I had to choose I would probably pick 3.9 or 3.10.

@tfmoraes
Copy link
Member

@paulojamorim can you try a more recent Ubuntu version like 24.04 (it's LTS)? Also, you can try to force the use of Zink OpenGL driver (it's a OpenGL implemented above Vulkan, it'll use the Vulkan from your driver, it's very fast). Try this:

__GLX_VENDOR_LIBRARY_NAME=mesa MESA_LOADER_DRIVER_OVERRIDE=zink GALLIUM_DRIVER=zink python app.py

@tfmoraes
Copy link
Member

@omar-abdelgawad I think 3.10 is a good minimum.

@okahilak
Copy link
Collaborator Author

We are using InVesalius with ROS2, which only officially supports Python 3.8 (see https://docs.ros.org/en/iron/Installation/Windows-Install-Binary.html). I don't know if more recent Python versions work or not - it could be tested, but I have very limited time to look into it at the moment.

@okahilak
Copy link
Collaborator Author

okahilak commented May 20, 2024

Oh, we also use MATLAB's ROS toolbox, which has some features that specifically demand a Python version 3.8 -- it gives an error if trying to use it with later versions.

The bottom line is that the Python community is notoriously slow at adopting new versions (even after end-of-life of a particular version), and there are probably many users who need to work with Python software and libraries that may not support newer versions. Dropping Python 3.8 support would mean that those users would then need to manage multiple different Python versions, which is always a hassle.

For those reasons, I wouldn't drop Python 3.8 support unless there are significant benefits to it. Maybe a longer transition period, during which we could investigate how to transition to newer Python versions in our project, would be appropriate?

@tfmoraes
Copy link
Member

Python 3.8 final support is in october 2024. We can try to update the minimum version later. Thanks @okahilak.

@okahilak
Copy link
Collaborator Author

okahilak commented May 20, 2024

Thanks for the comment, @tfmoraes - I wouldn't close this issue, though, as the main branch does not currently work with Python 3.8, causing ongoing trouble in our project until the support for Python 3.8 is brought back.

@okahilak okahilak reopened this May 20, 2024
@tfmoraes
Copy link
Member

tfmoraes commented May 20, 2024 via email

@okahilak
Copy link
Collaborator Author

okahilak commented May 20, 2024

Hmm, I think you refer to this commit, pushed directly to the master:

commit 6fff9fe7bea7622f0f69caaab331158668e931f8 (HEAD, origin/master, origin/HEAD)
Author: Thiago Franco de Moraes <totonixsame@gmail.com>
Date:   Mon May 20 10:28:23 2024 -0300

    Closes #773: using Tuple instead of tuple

I didn't notice it as it didn't go through the usual pull request-merge workflow. I'm re-closing the issue, as that change indeed seems to fix it. In general, it would be more transparent and easier to track and follow changes if all the changes went through the usual workflow - even if the author of the change merged it themselves.

In any case, thanks for fixing it!

@tfmoraes
Copy link
Member

@okahilak that commit not fixed all problems with python 3.8. I submitted a new commit that fixed the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants