Skip to content

Conversation

@jo-mueller
Copy link
Contributor

Fixes #13

Hi @m-albert ,

the changes to bring this feature into the plugin were actually quite minimal :) I took the liberty of renaming the binning option in the advanced settings tab so that they'd be valid for both registration and fusion. Otherwise, unlike the registration call, the fuse command required the target spacing in all axes, so I had to gather the z-spacing and paste it into the output_spacing.

Let me know what you think :)

@codecov
Copy link

codecov bot commented Jul 7, 2025

Codecov Report

Attention: Patch coverage is 44.44444% with 5 lines in your changes missing coverage. Please review.

Project coverage is 81.09%. Comparing base (f9f950e) to head (cf545e9).
Report is 4 commits behind head on main.

Current head cf545e9 differs from pull request most recent head b5b4bf6

Please upload reports for the commit b5b4bf6 to get more accurate results.

Files with missing lines Patch % Lines
src/napari_stitcher/_stitcher_widget.py 44.44% 5 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (f9f950e) and HEAD (cf545e9). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (f9f950e) HEAD (cf545e9)
4 3
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #15      +/-   ##
==========================================
- Coverage   86.24%   81.09%   -5.15%     
==========================================
  Files          13       13              
  Lines         858      873      +15     
==========================================
- Hits          740      708      -32     
- Misses        118      165      +47     

☔ View full report in Codecov by Sentry.
📢 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.

@jo-mueller
Copy link
Contributor Author

Edit: Just added a test, ran locally and realized that tests are probably running only on ubuntu and macOS for a reason. Committing and 🤞 that it's working in the CI...

Copy link
Collaborator

@m-albert m-albert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jo-mueller thanks a lot for this PR! Added a few comments.

@jo-mueller
Copy link
Contributor Author

@m-albert Should hopefully be alright now 👍

@m-albert
Copy link
Collaborator

Thanks for your work here @jo-mueller. Will have a look at this next week!

@jo-mueller
Copy link
Contributor Author

No hurries, it's not at all time-critical. Enjoy your vacations!

Copy link
Collaborator

@m-albert m-albert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jo-mueller, sorry for the delay in reviewing here.

Thanks again for the PR! I reviewed and left some comments on the changes.


# check which keys are in spacing that are missing in fusion_binning and add them
if self.custom_fuse_binning.value:
fusion_binning = {'y': self.x_fuse_binning.value, 'x': self.x_fuse_binning.value}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably this would be self.y_fuse_binning.value for y!

fusing_spacing = spatial_image_utils.get_spacing_from_sim(sims[0])
fusing_spacing = {
key: fusing_spacing[key] * fusion_binning[key]
for key in fusing_spacing.keys() if key in fusion_binning
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of 3D input data, fusing_spacing would currently loose the 'z' key (I tink best is to just keep it).

stitcher_widget.run_fusion()

# turn on custom binning
stitcher_widget.custom_reg_binning.value = True
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be great to iterate through both reg and fus binning on and off in the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: Option to subsample?

2 participants