Create skeleton for napari plugin with collapsible widgets

[niksirbi]

Description

What is this PR

• Bug fix
• Addition of a new feature
• Other

Why is this PR needed?

As explained in #31, I've shifted strategy for developing the napari plugin. This is the first in a series of multiple PRs, in which I'll try breaking down the work-in-progress contained in #112, and debug any obstacles I encounter along the way.
The PRs will be merged into the napari-dev branch, until the plugin becomes minimally functional for users, at which point we'll merge the napari-dev branch into main.

What does this PR do?

It sets up all the necessary components for a napari plugin, including optional dependencies, the napari.yaml file, a simple collapsible widget (which for now just includes a simple "hello, world" button as a placeholder), and some rudimentary smoke tests.

I struggled with finding the right way to install napari, such that it works across OSes (both locally and in CI). While debugging this, I stumbled on an issue with the brainglobe-utils.

In the end, pip install napari[all] seems to have worked across the platforms we test. For some reason, it doesn't work on recent Ubuntu versions (like my local Ubuntu 24.04), but I think I'm going to ignore this issue for now, open an issue about it, and wait for napari to deal with it.

The optional dependencies included in movement[napari] are :

napari = [
  "napari[all]>=0.4.19",
  # the rest will be replaced by brainglobe-utils[qt]>=0.6 after release
  "brainglobe-atlasapi>=2.0.7",
  "brainglobe-utils>=0.5",
  "qtpy",
  "superqt",
]

but, after brainglobe/brainglobe-utils#84 is merged and a new version of brainglobe-utils is released, that can change to just napari = ["napari[all]>=0.4.19", brainglobe-utils[qt]>=0.6"].

The installation instruction and tox configuration have been updated accordingly (see sections on testing and docs below).

References

#31
#112
brainglobe/brainglobe-utils#84

How has this PR been tested?

Tests have been added for the napari widget. The tox config has been modified to pass the right env variables for QT tests to work headlessly in CI. Additionally, an extra action has been added to the test workflow to facilitate this.

Is this a breaking change?

No.

Does this PR require an update to the documentation?

Yes, and the installation instructions have been updated both in the README and the more detailed installation guide in the docs.

Checklist:

• The code has been tested locally
• Tests have been added to cover all new functionality
• The documentation has been updated to reflect any changes
• The code has been formatted with pre-commit

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.72%. Comparing base (a840494) to head (0dcf6f3).

Additional details and impacted files

@@             Coverage Diff             @@
##           napari-dev     #218   +/-   ##
===========================================
  Coverage       99.71%   99.72%           
===========================================
  Files              13       15    +2     
  Lines             702      726   +24     
===========================================
+ Hits              700      724   +24     
  Misses              2        2           

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

[niksirbi]

tests fail on windows-latest runners with:

FAILED tests/test_unit/test_filtering.py::test_savgol_filter_with_nans - numpy.linalg.LinAlgError: SVD did not converge in Linear Least Squares

That's perplexing because this doesn't occur on any other OS, and also not on Windows on our other branches. My only guess is that the new dependencies cause us to end up with different versions of numpy (or with its conda-forge version instead of the pip version), and this old issue may have reared its head (though that should have been patched in 2021).

Maybe pinning numpy>1.20 will help?

[niksirbi]

Maybe pinning numpy>1.20 will help?

That's not it. I tried pinning napari>=0.4.19 which in turn pins numpy>=1.22, but I get the same exact error.

[niksirbi]

tests fail on windows-latest runners

In the end this problem disappeared after I switched back to a pip installation of napari 🤷🏼‍♂️

[alessandrofelder]

I have locally played with the napari plugin and works as expected
Tests pass locally too.

Code changes look reasonable to me - one suggestion going forwards around GUI test design.

[alessandrofelder]

This is probably overkill right now, but will become important as the plugin grows.

You've split the functionality from the GUI code well in the loader widget (which is IMO good practice), but not in the test. I would consider (at least going forwards) doing the same for the tests - so if you change just one of the two things, you only adapt the related tests.

In other words

• write a unit test that checks that if you click the button, a mock _on_hello_clicked is called.
• write a unit test that checks that if you execute _on_hello_clicked, the expected message is written to sysout.

Happy to have a pair-programming session to figure out the mocking, which is always a bit tricky, if you like 😄

[niksirbi]

That's a great suggestion, thanks! I'm willing to do this in this PR (even if it's a toy example) so that I establish a pattern I can keep using in the future.

I'll give it shot on my own, and will ping you to double-check. We can do pair-programming if I'm completely baffled by the mocking.

[niksirbi]

I've attempted to split the above test as follows:

from unittest.mock import patch

def test_hello_button_calls_on_hello_clicked(loader_widget):
    """Test that clicking the hello button calls _on_hello_clicked."""
    hello_button = loader_widget.findChildren(QPushButton)[0]
    with patch.object(loader_widget, "_on_hello_clicked") as mock_method:
        hello_button.click()
        mock_method.assert_called_once()


def test_on_hello_clicked_outputs_message(loader_widget, capsys):
    """Test that _on_hello_clicked outputs the expected message."""
    loader_widget._on_hello_clicked()
    captured = capsys.readouterr()
    assert "INFO: Hello, world!" in captured.out

but the first test keeps failing with:

E           AssertionError: Expected '_on_hello_clicked' to have been called once. Called 0 times.}

I've also tried pytest.mocker instead of unittest but I keep stumbling on the same problem.
Am I fundamentally misunderstanding something about mocking?
If the solution isn't obvious we can also pair program at some point next week.

[alessandrofelder]

FWIW, I've played around with this, and the code below works.
IIUC you need to create a new Loader object after mocking the function, because otherwise it's too late to mock (the loader_widget has already decided what function is connected to).

from movement.napari._loader_widget import Loader

def test_hello_button_calls_on_hello_clicked(make_napari_viewer, mocker):
    """Test that clicking the hello button calls _on_hello_clicked."""
    mock_method = mocker.patch("movement.napari._loader_widget.Loader._on_hello_clicked")
    loader = Loader(make_napari_viewer)
    hello_button = loader.findChildren(QPushButton)[0]    
    hello_button.click()
    mock_method.assert_called_once()

[niksirbi]

IIUC you need to create a new Loader object after mocking the function, because otherwise it's too late to mock (the loader_widget has already decided what function is connected to).

Thanks a lot @alessandrofelder! It would have taken me forever to figure this out on my own.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud