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

Qt 5.15.4 compatibility #2214

Merged
merged 3 commits into from
Oct 25, 2022
Merged

Qt 5.15.4 compatibility #2214

merged 3 commits into from
Oct 25, 2022

Conversation

llimeht
Copy link
Contributor

@llimeht llimeht commented Sep 21, 2022

This PR attempts to improve compatibility with newer Qt/PyQt releases. Several functions regarding progress bar values and pixel positioning that used to accept float now only accept int. The C++ Qt documentation for these functions uniformly says that the arguments should be int; I'm not sure whether the failures observed are due to changes in Qt or PyQt, and they may even be architecture-specific.

I expect that there are more changes like these needed, as I found these spots just by wandering through the interface loading dialogues and hitting "fit" or "compute" buttons.

Platform: Linux (Debian)
Python: 3.10
Qt: 5.15.7
PyQt: 5.15.4

@butlerpd butlerpd added the Discuss At The Call Issues to be discussed at the fortnightly call label Sep 26, 2022
@wpotrzebowski
Copy link
Contributor

I've tested running it from installer on Mac and SasView crashes on start. After some debugging the problem seems to be related to setuptools._distutils:

Traceback (most recent call last):
  File "sasview.py", line 13, in <module>
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "sas/qtgui/MainWindow/MainWindow.py", line 6, in <module>
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "sas/sasview/__init__.py", line 1, in <module>
    """Python bindings for 0MQ."""
  File "_distutils_hack/__init__.py", line 88, in create_module
    path_before = os.environ.get("PATH")
  File "importlib/__init__.py", line 127, in import_module
    
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "setuptools/__init__.py", line 10, in <module>
    from contextlib import contextmanager
  File "_distutils_hack/__init__.py", line 88, in create_module
    path_before = os.environ.get("PATH")
  File "importlib/__init__.py", line 127, in import_module
    
ModuleNotFoundError: No module named 'setuptools._distutils'
[83197] Failed to execute script 'sasview' due to unhandled exception: No module named 'setuptools._distutils'
[83197] Traceback:
Traceback (most recent call last):
  File "sasview.py", line 13, in <module>
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "sas/qtgui/MainWindow/MainWindow.py", line 6, in <module>
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "sas/sasview/__init__.py", line 1, in <module>
  File "_distutils_hack/__init__.py", line 88, in create_module
  File "importlib/__init__.py", line 127, in import_module
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "setuptools/__init__.py", line 10, in <module>
  File "_distutils_hack/__init__.py", line 88, in create_module
  File "importlib/__init__.py", line 127, in import_module
ModuleNotFoundError: No module named 'setuptools._distutils'

@wpotrzebowski
Copy link
Contributor

Maybe that's what it is fixed with #2216?

@llimeht
Copy link
Contributor Author

llimeht commented Sep 29, 2022

@wpotrzebowski, no, that's #2222 and unrelated to this PR. The solution is to either upgrade setuptools or upgrade matplotlib (or both).

@rozyczko
Copy link
Member

rozyczko commented Oct 3, 2022

Interestingly, on Windows, PyQt ver. 5.15.7 comes with Qt 5.15.2

PYQT_VERSION = 331527
PYQT_VERSION_STR = 5.15.7
QT_VERSION_STR = 5.15.2

The changes proposed seem to work fine, though. I assume when Windows Qt versioning catches up with Linux, all should be fine as well.

@wpotrzebowski wpotrzebowski added Discuss At The Call Issues to be discussed at the fortnightly call and removed Discuss At The Call Issues to be discussed at the fortnightly call labels Oct 4, 2022
@wpotrzebowski
Copy link
Contributor

@wpotrzebowski and @rozyczko will test it before merging

@wpotrzebowski wpotrzebowski removed the Discuss At The Call Issues to be discussed at the fortnightly call label Oct 11, 2022
@llimeht
Copy link
Contributor Author

llimeht commented Oct 20, 2022

Rebased to current main so that CI can regenerate the installers with the setuptools fixes in them.

@andyfaff
Copy link
Contributor

At some point the jump to PyQt6 might have to be made.

There are no PyQt5 wheels for macosx_arm64 on PyPI, although there are conda-forge packages for PyQt5 (as pyqt).
Conversely there are macosx_arm64 pyqt6 on PyPI, but no PyQt6 packages on conda-forge.

I would imagine this issue would become more pressing as more people transition to M series chips from x86_64. Do most users install dependencies via pip or conda?

@rozyczko
Copy link
Member

At some point the jump to PyQt6 might have to be made.

#2257

@wpotrzebowski wpotrzebowski self-requested a review October 25, 2022 09:58
@butlerpd
Copy link
Member

@rozyczko or @pbeaucage will look at this with a windows machine.

Copy link
Contributor

@pbeaucage pbeaucage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working on Windows.

@pbeaucage pbeaucage merged commit da859af into SasView:main Oct 25, 2022
@llimeht llimeht deleted the tmp/qt5154 branch October 26, 2022 01:29
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

Successfully merging this pull request may close these issues.

6 participants