jni / affinder

Quickly find the affine matrix mapping one image to another using manual correspondence points annotation
https://jni.github.io/affinder/
BSD 3-Clause "New" or "Revised" License
17 stars 13 forks source link

Consider scale and translation in apply_tf #82

Closed andreasmarnold closed 10 months ago

andreasmarnold commented 10 months ago

This addresses issue #80

Note: Affine transformations are not inherently commutative so I'm not sure this is universally correct. It seems to work but maybe you know a more elegant solution?

Unfortunately, I didn't quite remember how to do the auto-formatting without having pre-commit... Also, I'd be very grateful for any feedback on how well I adhered to best-practices in git (correct branch, etc...). I tried but it's all a bit new for me.

I did not change anything in the tests.

codecov-commenter commented 10 months ago

Codecov Report

Merging #82 (63bee56) into main (40370a1) will increase coverage by 0.35%. The diff coverage is 100.00%.

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

@@            Coverage Diff             @@
##             main      #82      +/-   ##
==========================================
+ Coverage   89.42%   89.78%   +0.35%     
==========================================
  Files           6        6              
  Lines         227      235       +8     
==========================================
+ Hits          203      211       +8     
  Misses         24       24              
Files Coverage Ξ”
src/affinder/apply_tf.py 100.00% <100.00%> (ΓΈ)
jni commented 10 months ago

Hey!

You should generally make a new branch from main and submit a PR from that branch. This is because otherwise your main branch diverges from the one in this project, which makes it harder to update in the future.

It's easy to go back though:

git reset HEAD~  # <-- point the main branch to the commit before this one
git switch --create apply-tf-complete  # <-- make a new branch with this name
git commit -a -m "take into account other layer transforms"  # <-- make your commit again, now on correct branch
git push origin --set-upstream apply-tf-complete  # <-- push new branch to your fork

Then you can (and should) make a new PR from that branch.

I wrote a git tutorial many years ago β€” you might find it useful. https://jni.github.io/git-tutorial

As to the rotations etc, you're right, it's tricky, which is why I didn't immediately make a PR to fix things. πŸ˜… But you shouldn't do it the way you've done it, instead we have a utility:

from napari.utils.transforms import CompositeAffine

that will take all the initial transforms and apply them for you in the right order, and make a matrix. However, I'm not 100% sure that I want to use it directly, because that means importing from napari. Maybe we can just copy the relevant code over.

PS: Also, after you make the PR you can fix your main branch by doing:

git switch main
git pull upstream  # <-- in case I made some changes
git push origin --force-with-lease  # overwrite the remote branch, but only if someone else hasn't changed it since the last time you pushed; not relevant here but good habit compared with --force
jni commented 10 months ago

I'll close this one because it's going to get borked anyway when you fix your main branch. πŸ˜…

jni commented 10 months ago

(And of course I'm happy to chat live if you encounter issues with git!)

andreasmarnold commented 10 months ago

Hey Juan, Thanks for the reply!

git reset HEAD~ # <-- point the main branch to the commit before this one git switch --create apply-tf-complete # <-- make a new branch with this name git commit -a -m "take into account other layer transforms" # <-- make your commit again, now on correct branch git push origin --set-upstream apply-tf-complete # <-- push new branch to your fork

I fixed this but I didn't get around to look into properly applying the transformations yet. I'll make a new PR once I've done some work on that (probably towards the end of this week).

And thanks for the infos on git! Really helpful :)