Skip to content

Add option to click corresponding points in a pairwise fashion before transform is first estimated#101

Merged
jni merged 4 commits intojni:mainfrom
m-albert:pairwise_mode
Jul 18, 2025
Merged

Add option to click corresponding points in a pairwise fashion before transform is first estimated#101
jni merged 4 commits intojni:mainfrom
m-albert:pairwise_mode

Conversation

@m-albert
Copy link
Copy Markdown
Contributor

@m-albert m-albert commented Jun 29, 2025

Implements the proposed pairwise clicking mode detailed in #100:

Would it be an option to consider including a mode in which points are added in a pairwise fashion? We were thinking this would not necessarily need to change the current clicking flow but could be an option provided in an additional drop-down menu. Optionally, there could be a notification showing up how many more points are required before the transform can be estimated.

Let me know what you think @jni! Happy to include any changes / improvements.

Screenshot:
image

Update

After incorporating @jni's improvements and at the final state of the PR the plugin looks like this:

image

@jni
Copy link
Copy Markdown
Owner

jni commented Jul 3, 2025

Ha! I love the test it's very clever. 😊

Even though the tooltip is very good, I think "pairwise point addition mode" is fairly obscure, especially since eventually all modes are pairwise point addition modes. 😂 My alternative suggestion is to use an enum:

Initial point annotation = alternating / grouped by layer

Tooltip: Before affinder can have an initial transform estimate, it needs ndim + 1 points to match between reference and moving layers. In alternating mode, you pick one point in reference, then its corresponding point in moving, and so on. In grouped mode, you first pick ndim + 1 points in the reference layer, then ndim + 1 in the moving layer in the same order, before alternating.

@m-albert
Copy link
Copy Markdown
Contributor Author

m-albert commented Jul 3, 2025

Thanks for your suggestions @jni which I completely agree with! Just pushed the changes.

@jni
Copy link
Copy Markdown
Owner

jni commented Jul 12, 2025

Thanks @m-albert! Would you mind appending an "Update" section to the original post with an up to date screenshot? That way the commit message will contain the picture corresponding to the final state of the PR. 🙏

@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.32%. Comparing base (ac135ce) to head (350cb9b).
Report is 3 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #101      +/-   ##
==========================================
+ Coverage   91.95%   92.32%   +0.37%     
==========================================
  Files           8        8              
  Lines         373      391      +18     
==========================================
+ Hits          343      361      +18     
  Misses         30       30              

☔ 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.

@m-albert
Copy link
Copy Markdown
Contributor Author

@jni Done!

@jni jni merged commit ea6b5f5 into jni:main Jul 18, 2025
8 checks passed
@jni
Copy link
Copy Markdown
Owner

jni commented Jul 21, 2025

This is now released as 0.5.0. 🥳

@m-albert
Copy link
Copy Markdown
Contributor Author

Amazing, thank you @jni!

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.

3 participants