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

Upgrade to Qt6.2 #23

Merged
merged 4 commits into from
Mar 18, 2022
Merged

Upgrade to Qt6.2 #23

merged 4 commits into from
Mar 18, 2022

Conversation

raimund-schluessler
Copy link
Contributor

@raimund-schluessler raimund-schluessler commented Mar 15, 2022

This PR upgrades PyQt5 to PyQt6 in order to support Macs with M1 Arm processor, which are supported natively from Qt6.2 on.

@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2022

Codecov Report

Merging #23 (90164c8) into main (fe839e6) will increase coverage by 0.01%.
The diff coverage is 76.47%.

@@            Coverage Diff             @@
##             main      #23      +/-   ##
==========================================
+ Coverage   77.63%   77.64%   +0.01%     
==========================================
  Files          35       35              
  Lines        2423     2425       +2     
==========================================
+ Hits         1881     1883       +2     
  Misses        542      542              
Impacted Files Coverage Δ
impose/__main__.py 0.00% <0.00%> (ø)
impose/gui/dlg_edit_shape.py 23.68% <50.00%> (ø)
impose/gui/main.py 65.97% <50.00%> (ø)
impose/gui/colocalize_pgrois.py 56.90% <75.00%> (ø)
impose/__init__.py 100.00% <100.00%> (ø)
impose/gui/collect.py 83.78% <100.00%> (ø)
impose/gui/collect_pgrois.py 82.81% <100.00%> (ø)
impose/gui/collect_shape_controls.py 62.37% <100.00%> (ø)
impose/gui/colocalize.py 56.66% <100.00%> (ø)
impose/gui/visualize.py 93.52% <100.00%> (ø)
... and 3 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@paulmueller
Copy link
Member

Could be related to pyqtgraph?

@raimund-schluessler
Copy link
Contributor Author

Could be related to pyqtgraph?

Yes, indeed. It turns out that pyqtgraph failed to determine that it should use PyQt6 and was silently still using PyQt5 in the background. Forcing PyQt6 in 90164c8 solved this problem.

@raimund-schluessler raimund-schluessler marked this pull request as ready for review March 16, 2022 10:27
@raimund-schluessler
Copy link
Contributor Author

This should be good to be reviewed now. As far as I can tell, everything still works as it should.

@@ -1,3 +1,7 @@
import os

os.environ["PYQTGRAPH_QT_LIB"] = "PyQt6"
Copy link
Member

Choose a reason for hiding this comment

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

This is only important if PyQt5 is installed, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tend to say yes. But pyqtgraph was very thorough in finding pyqt5 and found it even though it was not installed in the ENV running Impose, but in the global python environment. And since in the env only pyqt6 was available running Impose failed then.
I would like to leave this in, just to be sure pyqtraph uses pyqt6, because otherwise it will fail completely.

Copy link
Member

@paulmueller paulmueller left a comment

Choose a reason for hiding this comment

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

Thanks for the effort. Could you please update the changelog here as well (bump xyz from a to b)?

@raimund-schluessler
Copy link
Contributor Author

Thanks for the effort. Could you please update the changelog here as well (bump xyz from a to b)?

Done.

@paulmueller paulmueller merged commit 17102d8 into main Mar 18, 2022
@paulmueller paulmueller deleted the feature/noid/qt6 branch March 18, 2022 10:24
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.

3 participants