pymmcore-plus / pymmcore-widgets

A set of Qt-based widgets onto the pymmcore-plus model
https://pymmcore-plus.github.io/pymmcore-widgets
Other
12 stars 7 forks source link

feat: add minimal Points plan view #316

Closed tlambert03 closed 4 months ago

tlambert03 commented 4 months ago

borrowed and modified from #308

hey @fdrgsp, rather than editing each line of the existing FOVSelector, I decided to pull out the bits that I understood and make a bare minimal working widget. I'm sure that there are some features being missed here, but we can add them back as you point them out (lets be relatively strict about features though).

the main thing I did here was just to allow the GraphicsScene to use real-world coordinates. the qgraphics framework provides all sorts of conveniences in mapping between coordinate frames of the view and the scene, so to me it seems to make a lot of sense just to place the scene in the "real world" (using microns, just as useq schema positions will use), and then adjust the view as needed. (pen size is the one annoying thing here, it works as is here, but i think that could also be smarter... need to read more qt docs). I'll try to add bullet points of things that I know i've removed, and we can add them back, but you can also start a bulleted list as well.

would be great if you could play with this and tell me what you see

Screenshot 2024-07-09 at 4 59 28 PM
codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 98.67257% with 3 lines in your changes missing coverage. Please review.

Project coverage is 90.36%. Comparing base (a2b18b8) to head (e0336e3). Report is 30 commits behind head on main.

Files with missing lines Patch % Lines
src/pymmcore_widgets/_util.py 93.33% 1 Missing :warning:
...useq_widgets/points_plans/_points_plan_selector.py 97.77% 1 Missing :warning:
...useq_widgets/points_plans/_random_points_widget.py 92.30% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #316 +/- ## ========================================== + Coverage 90.07% 90.36% +0.28% ========================================== Files 71 73 +2 Lines 7930 8135 +205 ========================================== + Hits 7143 7351 +208 + Misses 787 784 -3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

fdrgsp commented 4 months ago

https://github.com/pymmcore-plus/pymmcore-widgets/blob/4c515835af5ddca78205540040eacd84f13137fb/src/pymmcore_widgets/useq_widgets/points_plans/_points_plan_selector.py#L132-L145

I think at the end here we need to add self.valueChanged.emit(self.value()) otherwise, if the radio button is not changing , the setChecked method won't trigger the update of the WellView...

~I believe this will also solve this~

there is a minor bug where if you extend the bounds of the scene, you can't "unextend" them for the purpose of autoresizing. for example, use a grid plan, and add more rows/columns than fit into the coverslip area. The view will (nicely) resize itself to show the full scene... however, if you then reduce the number of rows/cols, it doesn't "recover" and resize back to the usual size

no it does not :)

tlambert03 commented 4 months ago

added a line , and made it so that if the plan can't generate enough points, it suppresses the usual warning and just caps the widget

https://github.com/pymmcore-plus/pymmcore-widgets/assets/1609449/95084203-f427-4ac4-a010-9d64a3ef1352

tlambert03 commented 4 months ago

ok, i think this is getting closer. remaining issues above are

i think the first point can come later, the second is also optional. @fdrgsp, if you want to take a more critical look here at what has been done (knowing we can add more in future PRs) that would be great

tlambert03 commented 4 months ago

ok, I think all the things I wanted to deal with (besides tests) are now dealt with. Take another go at using it, and let me know how it feels. then I'll bring over the tests you wrote for this part, and fix them to work with any changes

tlambert03 commented 4 months ago

ok, tests added. Assuming they pass, feel free to merge this when ready @fdrgsp, and add the further improvements in a followup