Add option to scale sample image #26

IgorTatarnikov · 2024-01-17T13:19:03Z

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?
The plugin didn't have an option to handle images that are of a different resolution to the atlas.

What does this PR do?
There are now fields to enter the pixel size of the sample image and a button that downsamples it to the atlas resolution.

References

#19

How has this PR been tested?

Tests have been added to cover new functionality.

Is this a breaking change?

No.

Does this PR require an update to the documentation?

No.

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

codecov · 2024-01-17T13:21:36Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (fdec546) 82.08% compared to head (5495e75) 83.05%.

❗ Current head 5495e75 differs from pull request most recent head 0e4602c. Consider uploading reports for the commit 0e4602c to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##              dev      #26      +/-   ##
==========================================
+ Coverage   82.08%   83.05%   +0.96%     
==========================================
  Files           9        9              
  Lines         402      425      +23     
==========================================
+ Hits          330      353      +23     
  Misses         72       72

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

alessandrofelder

Generally code changes look good and are very clear - made some tiny comments.
Joint local testing suggests double-checking manually is a worthy next step?

tests/test_adjust_moving_image_view.py

brainglobe_registration/registration_widget.py

tests/test_registration_widget.py

…gistration_widget.py

IgorTatarnikov · 2024-02-05T14:27:15Z

I've now cached the original moving image data to allow for multiple rescaling operations to be completed. I've also disallowed pixel sizes of 0 to prevent undefined behaviour.

alessandrofelder

Manual testing suggest this works nice and smoothly, and as expected, now 🎉
Some tiny final comments to take or leave :)

alessandrofelder · 2024-02-05T14:46:28Z

brainglobe_registration/widgets/adjust_moving_image_view.py

@@ -58,9 +58,11 @@ def __init__(self, parent=None):
        rotation_range = 360

        self.adjust_moving_image_voxel_size_x = QDoubleSpinBox(parent=self)
-        self.adjust_moving_image_voxel_size_x.setDecimals(5)
+        self.adjust_moving_image_voxel_size_x.setDecimals(2)
+        self.adjust_moving_image_voxel_size_x.setMinimum(0.01)


(not sure) Should we allow the maximum to be > the default 99.99 (given that we have a 100um atlas)?

We can set it to be 100.00 just to catch that edge case, I don't think anyone will be acquiring anything at that coarse of a resolution but it's good to be prepared just in case.

alessandrofelder · 2024-02-05T14:48:07Z

brainglobe_registration/widgets/adjust_moving_image_view.py

-        self.layout().addRow(QLabel("Adjust the moving image: "))
+        self.layout().addRow(QLabel("Adjust the moving image scale:"))
+        self.layout().addRow(
+            "Sample image X pixel size:", self.adjust_moving_image_voxel_size_x


Suggested change

"Sample image X pixel size:", self.adjust_moving_image_voxel_size_x

"Sample image X pixel size [\u03BCm]:", self.adjust_moving_image_voxel_size_x

Worth specifying micron as unit for clarity?

Definitely, great suggestion!

* 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 added 5 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

IgorTatarnikov and others added 3 commits January 18, 2024 11:21

Merge branch 'dev' into scale-moving-image-resolution

ba3ab60

Added tests for scaling

b5b760b

Added tests to check multiple scale_factors

5495e75

IgorTatarnikov marked this pull request as ready for review January 18, 2024 16:31

IgorTatarnikov assigned alessandrofelder Jan 18, 2024

IgorTatarnikov assigned alessandrofelder and unassigned alessandrofelder Feb 2, 2024

alessandrofelder self-requested a review February 2, 2024 15:24

alessandrofelder reviewed Feb 2, 2024

View reviewed changes

tests/test_adjust_moving_image_view.py Outdated Show resolved Hide resolved

brainglobe_registration/registration_widget.py Show resolved Hide resolved

tests/test_registration_widget.py Outdated Show resolved Hide resolved

IgorTatarnikov added 5 commits February 5, 2024 10:53

Cache the moving image data to allow scaling image multiple times

8095c70

Fixed tests by removing (0,0) case add guard in _on_scale_image in re…

2c1fca2

…gistration_widget.py

Fixed tests

e61b07b

Add docstring to _on_scale_moving_image

fa9ee0f

Replaced all curr_ variable names with current_

0e4602c

IgorTatarnikov requested a review from alessandrofelder February 5, 2024 14:35

alessandrofelder approved these changes Feb 5, 2024

View reviewed changes

Added units to pixel size selection, set max range to be 100

3ad1639

IgorTatarnikov merged commit a09d4e6 into dev Feb 5, 2024
19 checks passed

IgorTatarnikov deleted the scale-moving-image-resolution branch February 5, 2024 15:44

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

Add option to scale sample image #26

Add option to scale sample image #26

IgorTatarnikov commented Jan 17, 2024 •

edited

Loading

codecov bot commented Jan 17, 2024 •

edited

Loading

alessandrofelder left a comment

IgorTatarnikov commented Feb 5, 2024

alessandrofelder left a comment

alessandrofelder Feb 5, 2024

IgorTatarnikov Feb 5, 2024

alessandrofelder Feb 5, 2024

IgorTatarnikov Feb 5, 2024

	"Sample image X pixel size:", self.adjust_moving_image_voxel_size_x
	"Sample image X pixel size [\u03BCm]:", self.adjust_moving_image_voxel_size_x

Add option to scale sample image #26

Add option to scale sample image #26

Conversation

IgorTatarnikov commented Jan 17, 2024 • edited Loading

Description

References

How has this PR been tested?

Is this a breaking change?

Does this PR require an update to the documentation?

Checklist:

codecov bot commented Jan 17, 2024 • edited Loading

Codecov Report

alessandrofelder left a comment

Choose a reason for hiding this comment

IgorTatarnikov commented Feb 5, 2024

alessandrofelder left a comment

Choose a reason for hiding this comment

alessandrofelder Feb 5, 2024

Choose a reason for hiding this comment

IgorTatarnikov Feb 5, 2024

Choose a reason for hiding this comment

alessandrofelder Feb 5, 2024

Choose a reason for hiding this comment

IgorTatarnikov Feb 5, 2024

Choose a reason for hiding this comment

IgorTatarnikov commented Jan 17, 2024 •

edited

Loading

codecov bot commented Jan 17, 2024 •

edited

Loading