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

Switch out magicgui in favour of QtWidgets #350

Merged
merged 42 commits into from
Aug 2, 2023

Conversation

paddyroddy
Copy link
Collaborator

@paddyroddy paddyroddy commented Jul 26, 2023

Fixes #352

This is needed as the first step in #266

@deprecated-napari-hub-preview-bot
Copy link

deprecated-napari-hub-preview-bot bot commented Jul 27, 2023

Preview page for your plugin is ready here:
https://preview.napari-hub.org/quantumjot/btrack/350
Updated: 2023-08-01T15:56:49.431214

@p-j-smith p-j-smith marked this pull request as ready for review August 1, 2023 14:35
@p-j-smith
Copy link
Collaborator

I think it's working!

The plugin UI looks mostly the same as before, and (as far as I cant tell) behaves exactly the same. The only visual differences are:

  • the widgets have been grouped into different sections (using QGroupBox)
  • the widgets are now centered rather than left-aligned.

plugin using magicgui:

magicgui-plugin

plugin using QtPy directly:

qtpy-plugin

The main internals changes are:

  • we're using QtPy directly rather than magicgui
  • added a BtrackWidget that inherits from QScrollArea and replaces the magicgui.widgets.Container we were using

@quantumjot
Copy link
Owner

I'm having a quick play with it and it looks great. I've noticed an error, when swapping between cell and particle tracking modalities:

        btrack_widget = <btrack.napari.widgets.create_ui.BtrackWidget object at 0x131f97f40>
        config.max_search_radius = 10.0
        btrack_widget.max_search_radius = <PyQt5.QtWidgets.QSpinBox object at 0x12612b520>
     85 # Update widgets from MotionModel values
     86 motion_model = config.motion_model

TypeError: setValue(self, val: int): argument 1 has unexpected type 'float'

@codecov-commenter
Copy link

codecov-commenter commented Aug 1, 2023

Codecov Report

Patch coverage: 94.98% and project coverage change: +0.76% 🎉

Comparison is base (0d8b714) 85.26% compared to head (1513897) 86.02%.
Report is 9 commits behind head on main.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #350      +/-   ##
==========================================
+ Coverage   85.26%   86.02%   +0.76%     
==========================================
  Files          30       30              
  Lines        1975     2097     +122     
  Branches      301      316      +15     
==========================================
+ Hits         1684     1804     +120     
- Misses        209      213       +4     
+ Partials       82       80       -2     
Files Changed Coverage Δ
btrack/napari/constants.py 100.00% <ø> (ø)
btrack/napari/main.py 70.43% <76.59%> (-2.90%) ⬇️
btrack/napari/sync.py 93.75% <95.23%> (+2.26%) ⬆️
btrack/napari/widgets/create_ui.py 98.61% <98.52%> (+12.89%) ⬆️
btrack/napari/widgets/__init__.py 100.00% <100.00%> (ø)
btrack/napari/widgets/_general.py 100.00% <100.00%> (+7.40%) ⬆️
btrack/napari/widgets/_hypothesis.py 100.00% <100.00%> (+3.77%) ⬆️
btrack/napari/widgets/_motion.py 100.00% <100.00%> (+8.69%) ⬆️

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

@p-j-smith
Copy link
Collaborator

I've noticed an error, when swapping between cell and particle tracking modalities:

huh, it's strange that I didn't get that error, but it should be sorted now - I've changed max_search_radius to use a QDoubleSpinBox

Copy link
Owner

@quantumjot quantumjot left a comment

Choose a reason for hiding this comment

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

Looks good to me! I suspect we'll find some other things as we use it more, but I don't want to impede the changeover.

@paddyroddy
Copy link
Collaborator Author

I've had a play and seems to be all working now. As you said @quantumjot, there might be issues, but it is expected as this was a huge piece of work. I will quickly read over the changes once more and merge.

@@ -26,5 +16,4 @@
"dist_thresh",
"time_thresh",
"apop_thresh",
"relax",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm going to merge this now, but just to check. @p-j-smith relax got removed because it should have never been in this list, right?

@paddyroddy paddyroddy merged commit 61f00a1 into main Aug 2, 2023
12 checks passed
@paddyroddy paddyroddy deleted the switch-magicgui-to-qwidgets branch August 2, 2023 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use QtPy to create the btrack napari plugin widgets rather than magicgui
4 participants