pyqt scatter widget

[fjorka]

Description

This PR introduces a complete Scatter Widget based on PyQtGraph, intended to replace the Matplotlib-based Scatter Widget.

Rationale

• Expected improved performance for large datasets.
• Smooth navigation for zooming and panning within the plot.
• Increased flexibility in customization.

Functionality

• It preserves all functionality of the previous widget.
• Documentation notebooks updated with the information and screenshots of the new widget.
• New functionality includes:
  • Separate color controls for continuous and discrete data:
    • Continuous color control offers:
      • Histogram of values chosen for color.
      • Ability to change LUT.
      • Control over contrast.
    • Discrete color control offers:
      • Scrollable legend of colors.
      • Ability to change the color for a selected class.
  • Pseudohistogram plot when only one axis is defined.
  • Highlight of nearby data points under cursor.
  • Dynamic status display of cursor position and highlighted data points.
  • Expanded ROI annotations:
    • Lasso or rectangle tool (useful for thresholding).
    • Ability to use several disjointed ROIs for annotation.
    • Modifiable ROIs.
  • Dialog to choose Annotation name (previously Export).
  • Saving options:
    • When started with AnnData, the Save button opens a path dialog to choose a location for an AnnData file (supports h5ad, zarr, and csv).
    • When opened with the Napari viewer, it mimics the behavior of the Annotation Widget:
      • With backed sdata, Save will modify sdata of the selected layer.
      • Without backed sdata, no saving action.
    • The widget displays the status and saving possibility when started.

Next Steps Beyond This PR

• [ ] Flip the Y-axis (scatterplot in spatial coordinates is flipped relative to the image).
• [ ] Signal point highlight to the main viewer for quick connection between visualizations in different dimensions.
• [ ] Highlight object in the main Napari window to trigger highlight in the scatterplot.
• [ ] Integrate Annotation Widget functionality into the Scatter Widget (the Annotation Widget facilitates annotating multiple classes within a single column).
• [ ] Performance improvements (1. How points belonging to polygons is checked.)

[review-notebook-app[bot]]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[melonora]

oh I see a green status! Ready for review?

[fjorka]

oh I see a green status! Ready for review?

Not yet, almost :)

[melonora]

Thanks @fjorka, I was trying it out on a windows machine. For now I seem to have caught a bug when loading the widget and then moving the mouse over it: AttributeError: 'QPointF' object has no attribute 'localPos', happening with the call of super().mouseMoveEvent. Will look into it.

[LucaMarconato]

Trying this now 😊 @melonora I get the same error on macOS.

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

LucaMarconato commented on 2024-08-15T14:02:13Z ----------------------------------------------------------------

Line #8.    #FILE_PATH = "../../../data/cosmx/data.zarr" # Change this

I'd delete this commented line.

fjorka commented on 2024-08-26T15:42:07Z ----------------------------------------------------------------

I standardized the initialization and data loading instructions across all notebooks.

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

LucaMarconato commented on 2024-08-15T14:02:13Z ----------------------------------------------------------------

Minor: please notice that "points 23" should be without space.

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

LucaMarconato commented on 2024-08-15T14:02:14Z ----------------------------------------------------------------

Minor typo on "annoation".

[LucaMarconato]

Here is a first initial round of review. I start with a review of the interface by trying it out (I didn't check the code yet), and of the documentation notebooks.

• In my latest commit I made a fix for the bug affecting me and Wouter.
• in my macOS interface I don't see the lasso and rectangle buttons 🥸 I can dig more into this. Edit: fixed, it was a problems with paths.
• [x] The notebooks look great! I would only make one thing clearer in the "scatterwidget.ipynb" and "scatterwidget_annotation.ipynb" notebooks. I would clarify a bit around the scenarios: "napari is running" vs "no napari viewer detected" and the case in which the table is from a standalone AnnData vs coming from a SpatialData object. In principle there are 4 cases. Are these two possible: "napari is running" and "the table is from a standalone AnnData" and "no napari viewer detected" and "the table is from a SpatialData object?
• [x] is it possible to remove the white border around the scatterplot points?
• [x] scatterwidget_annotation.ipynb is missing from docs/index.md
• [x] Minor thing, can you please update the changelog? Thanks.

I'll proceed next with the review of the code 😊 Looking forward to have this in main, super cool work!

[LucaMarconato]

Segfault in the tests 👀 It doesn't happen locally.

[LucaMarconato]

I re-run the tests, now we get a proper error (not a segfault):

  tests/test_scatterwidgets.py::test_add_roi_to_plot PASSED                [ 31%]
  tests/test_scatterwidgets.py::test_remove_roi_double_click PASSED        [ 32%]
  tests/test_scatterwidgets.py::test_remove_roi_double_click ERROR         [ 32%]
  tests/test_scatterwidgets.py::test_remove_hovered_roi PASSED             [ 33%]

Update: I discovered that the rectangle and lasso buttons are present, they are just black in macOS:

[LucaMarconato]

Ok I finished testing the interface, to be organized I suggest to keep action items clearly discoverable by proceeding as follow:

• tasks are either in the form of conversations; until Resolve conversation is pressed, the task is open
• [x] either in the form of check box items, like this one
• either posted automatically as comments from review-notebook-app, in that case we can consider the task open until a 👍 is given.

Also, I suggest to try to self-assign/split some of the tasks. I'll list mine below:

• [x] (Luca) test of the interface (didn't look at the code yet)
• [x] (Luca) review the notebooks
• [ ] fix the test with python 3.9 (see discussion below from Czaki)
• [x] (Luca) fix the problem with the lasso and rectangle selection button that appear as black in macOS.
• [x] (Luca) review the code

[Czaki]

In context of failing test. The widget created in the test is not either added to qtbot using qtbot.add_widget or docked into napari viewer so may be cleaned before all events were proceeded, that leads to segfault.

It may be impossible to reproduce on a local machine.

[Czaki]

You may want to read this discussion until we improve the documentation https://napari.zulipchat.com/#narrow/stream/212875-general/topic/qtbot.20documentation/near/462664015

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 83.26055% with 115 lines in your changes missing coverage. Please review.

Project coverage is 69.42%. Comparing base (69105e0) to head (7d60e03). Report is 39 commits behind head on main.

Files with missing lines	Patch %	Lines
src/napari_spatialdata/_view.py	60.00%	48 Missing :warning:
src/napari_spatialdata/_scatterwidgets.py	92.22%	41 Missing :warning:
src/napari_spatialdata/_widgets.py	35.00%	26 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #270      +/-   ##
==========================================
+ Coverage   63.08%   69.42%   +6.34%     
==========================================
  Files          19       19              
  Lines        2636     3235     +599     
==========================================
+ Hits         1663     2246     +583     
- Misses        973      989      +16     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: 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.

[LucaMarconato]

Thanks @Czaki for the explanation, I indeed can't reproduce the failing test on my machine with Python 3.9, and running the test again showed a segfault for a different test.

[LucaMarconato]

I checked, the test_remove_roi_double_click() (code here), which was giving me a segfault, seems to handle the qtbot correctly, as seen here.

Similarly, the test test_scatterlistwidget() (code here), which now is giving a segfault, seems to be calling make_napari_viewer() correctly (code here).

I'll move away from this issue for the moment and start reviewing the code and get back to this later. Meanwhile if anyone wants to take a look at this, you are welcome!

[fjorka]

• [x] is it possible to remove the white border around the scatterplot points?

Replaced with black (invisible) border when color is defined - 1acdd85274de92eebc5cb8607113bf4152cef3cc

• [x] scatterwidget_annotation.ipynb is missing from docs/index.md

Included in 9191c7e0ab91fafbc4d8559270ac6f5ada148f97

• [x] Minor thing, can you please update the changelog? Thanks.

Suggestion in 973ebcc3e36c9d293e4370a4a6b4c2c7d0672d73. Needs to be finalized.

[fjorka]

The notebooks start with the instruction:

There are two options to install napari-spatialdata:

(1) Run `pip install napari-spatialdata`
or,
(2) Clone this [repo](https://github.com/scverse/napari-spatialdata) and run `pip install -e .`

However, following this instruction and starting napari (or scatter widget) will give an error ImportError: No Qt bindings could be found. What about asking explicitly for napari installation first d40fb7cec410b6f8a0778999d210d81b28eeffca which will inform a user about Qt?

[fjorka]

To do:

• [x] add explanations in scatterwidget.ipynb and scatterwidget_annotation.ipynb about starting with AnnData and options for saving

There were some "next steps"/"open points" here. I moved them to this issue https://github.com/scverse/napari-spatialdata/issues/333 to make them more discoverable.

[LucaMarconato]

There were some "next steps"/"open points" here. I moved them to this issue https://github.com/scverse/napari-spatialdata/issues/333 to make them more discoverable.

[LucaMarconato]

Thanks @fjorka and @melonora, we finished reviewing and adjusting some last open points, and now we are good to merge!

Really excited to ultimately release this! We will include this PR in a release X.com post that we will make in the near future.

I will consolidate the points open for follow up PRs in a separate issue.

[LucaMarconato]

@fjorka @melonora I forgot about this file https://github.com/fjorka/napari-spatialdata/blob/7d60e030aff750ee08fd874b8c6b6425ec5fff79/tests/test_aaa.py, which if I got it right, was a work-in-progress implementation for an alternative approach to span the test napari viewer, in order to avoid the segfault problem with Python 3.9. I think the problem have disappeared due to having dropped the Python 3.9 support. So please @melonora in the follow up PR can you drop that file? Thanks.

[LucaMarconato]

Actually there is still a problem with tests, I reported it here https://github.com/scverse/napari-spatialdata/issues/335 so we can continue the discussion there. CC @melonora