embl-cba / elastixWrapper

13 stars 2 forks source link

ElastixWrapper refactoring #10

Closed K-Meech closed 3 years ago

K-Meech commented 3 years ago

@tischi this is a rather massive pull request - so happy to discuss/ restructure it if you'd like! All commands should remain the same, so there won't be any changes for users - only backend changes. The main aim was to separate the command line functionality so it could be called more easily from other code.

Main changes:

tischi commented 3 years ago

This looks really ❤️ ly ! Seriously great work!

I would just merge it as is. @K-Meech you can also do it (do you have the rights?). If you merge it please inform: https://forum.image.sc/t/exception-when-trying-to-run-elastix/30375 (He however did not seem to have forked it yet).

(@bogovicj @NicoKiaru: @K-Meech did a great job in making the code in this repo more modular, maybe this is also interesting for your usage, in case you are using the Java API of this repo, are you?)

NicoKiaru commented 3 years ago

This looks really great! I'm not using it for now, I could switch when I'll have time, but it's not on my priority list. . Probably when I'll need 3D registration, I'm going to 'reopen the case'.

K-Meech commented 3 years ago

Great!! @tischi I don't think I have the rights to merge - you'd have to add me as a collaborator.