Allow free rotation of the atlas #36

IgorTatarnikov · 2024-04-24T08:54:37Z

Before submitting a pull request (PR), please read the contributing guide.

Please fill out as much of this template as you can, but if you have any problems or questions, just leave a comment and we will help out :)

Description

What is this PR

Bug fix
Addition of a new feature
Other

Why is this PR needed?
Some slices may have been taken at an angle, rotating the atlas allows viewing and registering to non-orthogonal slices of the atlas.

What does this PR do?
Provides a UI to rotate the atlas based on yaw, pitch and roll. This allows the user to find the correct 2D slice to register to, even if it's not aligned with the atlas axes.

Checklist:

The code has been tested locally
Tests have been added to cover all new functionality (unit & integration)
The code has been formatted with pre-commit

…tion

…n to avoid race conditions

See neuroinformatics-unit/movement#108

…o avoid long boot times

updates: - [github.com/astral-sh/ruff-pre-commit: v0.1.9 → v0.2.0](astral-sh/ruff-pre-commit@v0.1.9...v0.2.0) - [github.com/psf/black: 23.12.1 → 24.1.1](psf/black@23.12.1...24.1.1) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

updates: - [github.com/astral-sh/ruff-pre-commit: v0.2.0 → v0.3.5](astral-sh/ruff-pre-commit@v0.2.0...v0.3.5) - [github.com/psf/black: 24.1.1 → 24.3.0](psf/black@24.1.1...24.3.0) - [github.com/pre-commit/mirrors-mypy: v1.8.0 → v1.9.0](pre-commit/mirrors-mypy@v1.8.0...v1.9.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* import header from brainglobe-utils * split package name over two lines * remove brainglobe png from manifest * add brainglobe-utils dependency

* import header from brainglobe-utils * split package name over two lines * remove brainglobe png from manifest * add brainglobe-utils dependency * Switched from bg-atlasapi to brainglobe-atlasapi * Added lxml_html_clean explicitly to requirements * Pinned itk to 5.4rc2 for now as 5.4rc3 cased a seg fault * Undo itk pin --------- Co-authored-by: Kimberly Meechan <24316371+K-Meech@users.noreply.github.com>

…e napari widget class

codecov · 2024-04-24T08:55:02Z

Codecov Report

Attention: Patch coverage is 89.28571% with 18 lines in your changes missing coverage. Please review.

Project coverage is 82.77%. Comparing base (0f32e54) to head (b129bd6).
Report is 2 commits behind head on dev.

Files	Patch %	Lines
brainglobe_registration/registration_widget.py	82.69%	18 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev      #36      +/-   ##
==========================================
+ Coverage   82.35%   82.77%   +0.42%     
==========================================
  Files           9        8       -1     
  Lines         408      511     +103     
==========================================
+ Hits          336      423      +87     
- Misses         72       88      +16

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

alessandrofelder

Thanks a lot @IgorTatarnikov and sorry for the delay in reviewing.
I made some minor comments related to legibility of the code. Looking really good!

I did some user testing and found UX improvements that might be worth addressing as separate issues (maybe you have already):

users can select a sample image, and then delete the layer in napari, and then cause an index error when selecting an image in the plugin.
the same can happen if you delete the atlas-related layers in napari and then select a new atlas in the plugin
if I click the rotate button in not-grid mode, it additionally toggles to grid mode - is that intended?

brainglobe_registration/registration_widget.py

tests/test_registration_widget.py

alessandrofelder · 2024-07-25T10:44:55Z

tests/test_registration_widget.py

+@pytest.mark.parametrize(
+    "pitch, yaw, roll, expected_shape",
+    [
+        (0, 0, 0, (132, 80, 114)),
+        (45, 0, 0, (150, 150, 114)),
+        (0, 45, 0, (174, 80, 174)),
+        (0, 0, 45, (132, 137, 137)),
+    ],
+)


Are the expected shape values a regression test, or have you cleverly calculated them?

Regression test based on visual inspection.

Co-authored-by: Alessandro Felder <alessandrofelder@users.noreply.github.com>

for more information, see https://pre-commit.ci

…astix into atlas-yaw-pitch

IgorTatarnikov · 2024-07-26T11:56:27Z

Thanks for taking a look!

I did some user testing and found UX improvements that might be worth addressing as separate issues (maybe you have already):
* users can select a sample image, and then delete the layer in napari, and then cause an index error when selecting an image in the plugin.

* the same can happen if you delete the atlas-related layers in napari and then select a new atlas in the plugin

This has been on my list of things to address, I'm not happy with how fragile it is at the moment. I haven't validated starting from "scratch" without re-opening napari. I'll make an issue.

* if I click the rotate button in not-grid mode, it additionally toggles to grid mode - is that intended?

This was a hacky way to reset the view, otherwise the rotated atlas doesn't quite look right. I'll come back to it and see if there's a better way of refreshing the canvas without toggling grid mode.

* Moving from tabs to a dropdown style widget (#25) * Fixed contents margins * Added brainglobe-utils as dependency * Added min dependency version for brainglobe-utils * Add option to scale sample image (#26) * Fixed contents margins * Scaling of sample image added to adjust moving image dropdown * Added brainglobe-utils as dependency * Added error message when no images selected * Added tests for scaling * Added tests to check multiple scale_factors * Cache the moving image data to allow scaling image multiple times * Fixed tests by removing (0,0) case add guard in _on_scale_image in registration_widget.py * Fixed tests * Add docstring to _on_scale_moving_image * Replaced all curr_ variable names with current_ * Added units to pixel size selection, set max range to be 100 * Allow free rotation of the atlas (#36) * Fixed contents margins * Scaling of sample image added to adjust moving image dropdown * Added brainglobe-utils as dependency * Added error message when no images selected * Atlas rotation working using scipy * Pitch, yaw, roll implemented using one affine transform * Dask loading for both the reference atlas and the rotation * Added a 'Reset Atlas' button, blocked while dask computes the rotation to avoid race conditions * Rotate atlas button also greyed out while daks processes the rotation * Use the update upload_pypi action (#29) See neuroinformatics-unit/movement#108 * Added elastix Logs directory to .gitignore * Fixed tests * Moved run_registration to be imported just as run button is clicked to avoid long boot times * Atlas rotation works again (might relate to napari 0.4.19 * Added test for atlas_rotation_signal from AdjustMovingImage view * [pre-commit.ci] pre-commit autoupdate (#30) updates: - [github.com/astral-sh/ruff-pre-commit: v0.1.9 → v0.2.0](astral-sh/ruff-pre-commit@v0.1.9...v0.2.0) - [github.com/psf/black: 23.12.1 → 24.1.1](psf/black@23.12.1...24.1.1) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * [pre-commit.ci] pre-commit autoupdate (#31) updates: - [github.com/astral-sh/ruff-pre-commit: v0.2.0 → v0.3.5](astral-sh/ruff-pre-commit@v0.2.0...v0.3.5) - [github.com/psf/black: 24.1.1 → 24.3.0](psf/black@24.1.1...24.3.0) - [github.com/pre-commit/mirrors-mypy: v1.8.0 → v1.9.0](pre-commit/mirrors-mypy@v1.8.0...v1.9.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Import header from brainglobe-utils (#33) * import header from brainglobe-utils * split package name over two lines * remove brainglobe png from manifest * add brainglobe-utils dependency * Update requirements (#34) * import header from brainglobe-utils * split package name over two lines * remove brainglobe png from manifest * add brainglobe-utils dependency * Switched from bg-atlasapi to brainglobe-atlasapi * Added lxml_html_clean explicitly to requirements * Pinned itk to 5.4rc2 for now as 5.4rc3 cased a seg fault * Undo itk pin --------- Co-authored-by: Kimberly Meechan <24316371+K-Meech@users.noreply.github.com> * add codecov token (#35) * Added tests for calculate_rotated_bounding_box * Fixed docstrings for functions in utils.py * Keep track of napari layers for the atlas and atlas annotations in the napari widget class * Added tests for atlas rotation * Fixed pre-commit * Add test for reset_atlas in adjust_moving_image_view * Add caching * Update the function calls in test_adjust_moving_image_view * Apply suggestions from code review Co-authored-by: Alessandro Felder <alessandrofelder@users.noreply.github.com> * Applied suggestions from code review * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Applied suggestions from code review --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Kimberly Meechan <24316371+K-Meech@users.noreply.github.com> Co-authored-by: Alessandro Felder <alessandrofelder@users.noreply.github.com> * Cleaned up a few things * Mark 'Advanced Settings' as optional (#48) * Info message when running BrainGlobe Registration plugin with no atlas installed (#46) * added error when no atlas found * added error when no atlas found * Update brainglobe_registration/registration_widget.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * Update brainglobe_registration/registration_widget.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * changes to info message * Update brainglobe_registration/utils/utils.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * Update brainglobe_registration/utils/utils.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * Update brainglobe_registration/utils/utils.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update brainglobe_registration/utils/utils.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * Update brainglobe_registration/utils/utils.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update brainglobe_registration/utils/utils.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * Update brainglobe_registration/utils/utils.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * Update brainglobe_registration/registration_widget.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * parent_widget * parent_widget --------- Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Clean up widget (#51) * Cleaned up rogue widget duplication * Added parent to parameters_tab * Fix slow plugin loading (#54) * Filter Images (#52) * work_in_progress * filter_images * filter_images * filter_images * filter_images * Update brainglobe_registration/utils/utils.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * Update brainglobe_registration/utils/utils.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * Update brainglobe_registration/utils/utils.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * Update brainglobe_registration/utils/utils.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Communicate napari layer deletions appropriately to the plugin (#49) * layer_deletion * Update brainglobe_registration/registration_widget.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * layer_deletion_edits * Made pytest happy * Update brainglobe_registration/registration_widget.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * Update brainglobe_registration/registration_widget.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * Update brainglobe_registration/registration_widget.py Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * updated_layer * fixed manual vs automatic deletion * fixed manual vs automatic deletion * Refactored some old code to avoid duplication * add comments --------- Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> Co-authored-by: IgorTatarnikov <igor.tatarnikov@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Kimberly Meechan <24316371+K-Meech@users.noreply.github.com> Co-authored-by: Alessandro Felder <alessandrofelder@users.noreply.github.com> Co-authored-by: saarah815 <63860766+saarah815@users.noreply.github.com>

IgorTatarnikov and others added 28 commits January 15, 2024 14:03

Fixed contents margins

1a8d415

Scaling of sample image added to adjust moving image dropdown

e71bbfb

Added brainglobe-utils as dependency

4bdcdf8

Merge branch 'move-to-dropdown-parent' into scale-moving-image-resolu…

20b97f7

…tion

Added error message when no images selected

35b24bd

Atlas rotation working using scipy

7d34970

Pitch, yaw, roll implemented using one affine transform

2b50fce

Dask loading for both the reference atlas and the rotation

8f42d33

Added a 'Reset Atlas' button, blocked while dask computes the rotatio…

94b38b6

…n to avoid race conditions

Rotate atlas button also greyed out while daks processes the rotation

fe109bf

Use the update upload_pypi action (#29)

7d17da1

See neuroinformatics-unit/movement#108

Resolved merge conflicts

5c5cd30

Added elastix Logs directory to .gitignore

d2bec0d

Fixed tests

9bd9ca4

Moved run_registration to be imported just as run button is clicked t…

8d4635b

…o avoid long boot times

Atlas rotation works again (might relate to napari 0.4.19

8480a9c

Added test for atlas_rotation_signal from AdjustMovingImage view

ae1110a

Import header from brainglobe-utils (#33)

ef2174d

* import header from brainglobe-utils * split package name over two lines * remove brainglobe png from manifest * add brainglobe-utils dependency

add codecov token (#35)

6436db4

Added tests for calculate_rotated_bounding_box

7767348

Fixed docstrings for functions in utils.py

9de8014

Keep track of napari layers for the atlas and atlas annotations in th…

78fbd78

…e napari widget class

Added tests for atlas rotation

4cbd6be

Fixed pre-commit

503f57e

Fixed merge conflicts

b172fd6

Add test for reset_atlas in adjust_moving_image_view

01db43b

IgorTatarnikov added 2 commits June 27, 2024 16:48

Add caching

e675ca0

Update the function calls in test_adjust_moving_image_view

a44410a

IgorTatarnikov marked this pull request as ready for review June 27, 2024 16:06

IgorTatarnikov requested a review from alessandrofelder June 27, 2024 16:06

alessandrofelder approved these changes Jul 25, 2024

View reviewed changes

IgorTatarnikov and others added 6 commits July 26, 2024 12:52

Apply suggestions from code review

8163101

Co-authored-by: Alessandro Felder <alessandrofelder@users.noreply.github.com>

Applied suggestions from code review

de3a281

[pre-commit.ci] auto fixes from pre-commit.com hooks

52a640f

for more information, see https://pre-commit.ci

Fixed merge conflicts

d5bee95

Applied suggestions from code review

680848b

Merge branch 'atlas-yaw-pitch' of https://github.com/brainglobe/bg-el…

b129bd6

…astix into atlas-yaw-pitch

IgorTatarnikov merged commit 1b664b6 into dev Jul 26, 2024
21 checks passed

IgorTatarnikov deleted the atlas-yaw-pitch branch July 26, 2024 12:30

IgorTatarnikov mentioned this pull request Aug 16, 2024

Update main wtih dev changes #55

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow free rotation of the atlas #36

Allow free rotation of the atlas #36

IgorTatarnikov commented Apr 24, 2024

codecov bot commented Apr 24, 2024 •

edited

Loading

alessandrofelder left a comment •

edited

Loading

alessandrofelder Jul 25, 2024

IgorTatarnikov Jul 26, 2024

IgorTatarnikov commented Jul 26, 2024

Allow free rotation of the atlas #36

Allow free rotation of the atlas #36

Conversation

IgorTatarnikov commented Apr 24, 2024

Description

Checklist:

codecov bot commented Apr 24, 2024 • edited Loading

Codecov Report

alessandrofelder left a comment • edited Loading

Choose a reason for hiding this comment

alessandrofelder Jul 25, 2024

Choose a reason for hiding this comment

IgorTatarnikov Jul 26, 2024

Choose a reason for hiding this comment

IgorTatarnikov commented Jul 26, 2024

codecov bot commented Apr 24, 2024 •

edited

Loading

alessandrofelder left a comment •

edited

Loading