Skip to content

Conversation

k-dominik
Copy link
Contributor

@k-dominik k-dominik commented Aug 28, 2025

In order to prepare enabling/switching to Pyside6 (qt6!) I propose to go via qtpy. This enables us to switch qt apis more flexibly. I already tested volumina with Pyside6. With this PR, this does not require any code changes (tested python 3.13 with pyside6). Note that the qt bindings with this pr are still PyQt5.

Summary:

  • change all PyQt5 import to qtpy
  • change use of qApp to QApplication.instance() (doesn't exist in pyside)
  • PyQt5.QtCore.pyqtSignal -> qtpy.QtCore.Signal
  • PyQt5.QtCore.pyqtSlot -> qtpy.QtCore.Slot
  • sip.isdeleted -> not qtpy.compat.isalive
  • small gymnastics for QUndoStack which is not agnostic to the in
    binding qtpy: (PyQt5, PySide2): QtWidgets; (PyQt6, PySide6): QtGui
  • Our QABC wouldn't fly anymore, I found a drop in replacement on stackoverflow.
  • Added qtpy to dependencies.
  • Updated min python (3.9), and the python used in conda build for tests (3.11)

Note: Should definitely be squash and merge for revertability.

* change all `PyQt5` import to `qtpy`
* change use of `qApp` to `QApplication.instance()` (doesn't exist in pyside)
* `PyQt5.QtCore.pyqtSignal` -> `qtpy.QtCore.Signal`
* `PyQt5.QtCore.pyqtSlot` -> `qtpy.QtCore.Slot`
* `sip.isdeleted` -> `not qtpy.compat.isalive`
* small gymnastics for `QUndoStack` which is not agnostic to the in
  binding `qtpy`: (PyQt5, PySide2): `QtWidgets`; (PyQt6, PySide6): QtGui
Copy link

codecov bot commented Aug 28, 2025

Codecov Report

❌ Patch coverage is 61.82336% with 134 lines in your changes missing coverage. Please review.
✅ Project coverage is 36.33%. Comparing base (eb00227) to head (3cbeba5).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
volumina/sliceSelectorHud.py 0.00% 15 Missing ⚠️
volumina/view3d/overview3d.py 0.00% 12 Missing ⚠️
volumina/quadsplitter.py 0.00% 6 Missing ⚠️
.../widgets/multiformatSlotExportFileOptionsWidget.py 0.00% 6 Missing ⚠️
volumina/view3d/meshgenerator.py 0.00% 5 Missing ⚠️
...ina/widgets/hierarchicalFileExportOptionsWidget.py 0.00% 5 Missing ⚠️
volumina/widgets/stackExportFileOptionsWidget.py 0.00% 5 Missing ⚠️
volumina/skeletons/skeletons.py 0.00% 4 Missing ⚠️
volumina/utility/shortcutManager.py 42.85% 4 Missing ⚠️
volumina/utility/shortcutManagerDlg.py 20.00% 4 Missing ⚠️
... and 35 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #333      +/-   ##
==========================================
+ Coverage   36.30%   36.33%   +0.02%     
==========================================
  Files         108      108              
  Lines       11424    11434      +10     
==========================================
+ Hits         4147     4154       +7     
- Misses       7277     7280       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@k-dominik k-dominik marked this pull request as ready for review August 28, 2025 07:25
@k-dominik k-dominik marked this pull request as draft August 28, 2025 08:06
@btbest
Copy link
Contributor

btbest commented Aug 29, 2025

Tried this together with the ilastik PR and seems ok I guess? Are you still planning changes?

@k-dominik
Copy link
Contributor Author

k-dominik commented Sep 8, 2025

really don't understand why the tests in the conda recipe time out...

Update: Seems to be a problem with cleaning up threads in the tests - I could not get to the root cause (I assume some changes in python) that would result in hangs when one of the tests was trying to .join some threads.

Added a fixture, where the threadpool is used implicitly and cleaned up after every test.

Not 100% sure about the root cause, but with Python 3.10, .join() on threads
from the global renderer pool would cause hangs.

This makes sure the tests that imageScene2D tests, clean up the render pool.
Lazyflow thread pool seems not to be affected by this problem.
@k-dominik k-dominik marked this pull request as ready for review September 8, 2025 12:51
@k-dominik k-dominik requested a review from btbest September 8, 2025 12:52
@btbest
Copy link
Contributor

btbest commented Sep 10, 2025

Tests green locally and sample projects load ok 👍 (ran with ilastik/ilastik#3068)

@k-dominik k-dominik merged commit e6d37a5 into ilastik:main Sep 11, 2025
20 checks passed
@k-dominik k-dominik deleted the qtpy branch September 11, 2025 05:49
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.

2 participants