search

DCBIA-OrthoLab / SlicerAutomatedDentalTools

A 3D Slicer extension to use AMASSS, ALI-CBCT and ALI-IOS
Other
79 stars 22 forks source link

Rebased #56 #57 - Fixed import issue of ASO and add input size option to AutoCrop3D #58

Closed allemangD closed 6 months ago

allemangD commented 6 months ago

The commit history in #56 and #57 is getting too difficult to follow. I suspect that one or both of @Jeanneclre or @GaelleLeroux upstream main branch is out-of-sync with the current upstream.

I've rebased the changes in #56 and #57 onto the current main branch. @Jeanneclre please checkout these changes and confirm that things behave as you expect. If they don't, feel free to push new commits to this PR - you should have write access to the rebased-autocrop-aso branch.

I suggest both of you reset your main branch to the current state - you can do this with commands (assuming the DCBIA-OrthoLab remote is called upstream):

git checkout main
git fetch --all
git reset --hard upstream/main

Then you should cherry-pick any new commits from the out-of-sync feature branch. For example, the way I created this PR is:

git checkout main
git checkout -b rebased-autocrop-aso
git cherry-pick f706183  # (the change from #56)
git cherry-pick 6defc0d  # (the change from #57)
git push -u upstream rebased-autocrop-aso

You can also do this with rebase --interactive if you have a suitable editor. Pick only the relevant commits.

See https://www.atlassian.com/git/tutorials/merging-vs-rebasing (there is also a French translation at https://www.atlassian.com/fr/git/tutorials/merging-vs-rebasing)

In general, once the commit history on all branches is in-sync, it is easier to avoid this situation by rebasing before creating each PR:

git fetch --all
git rebase upstream/main

Your editor may have a "rebase" option, but specific instructions vary from editor to editor. The important part is to make the commit history as straightforward as possible.

allemangD commented 6 months ago

Compare the commit history of #56 and #57

* 6defc0d (ASO) BUG: fixed import module ASO and "__init__" in slicer
| * f706183 (HEAD -> AutoCrop) ENH: check box to keep the original siize of the file
|/  
* 11b421c BUG: fixed import module ASO and "__init__" in slicer
* 6fd5394 ENH: check box to keep the original siize of the file
*   e67d954 Merge branch 'DCBIA-OrthoLab:main' into main
|\  
* | 06cdf0e DOC: updated link to trained model CBCT
* | 86b1d2d FIX: bounds checking and non-zero for dimension (CLI)
* | 8f032e2 BUG: Flybyprocess = autocrop3d_cli
| | * 8602659 (upstream/main, upstream/HEAD, main) Merge pull request #55 from GaelleLeroux/main
| |/| 
| | * 9c9b47d PERF : resample using exportNode
......................................................
| | * 7bdcb96 BUG : wrong interface cbct
| |/  
| *   e478693 Merge pull request #53 from GaelleLeroux/pytorch3d
| |\  
| | * df83de5 STYLE : delete old repository
......................................................
| | * 230d1cd STYLE : add print test branch
| * |   09462f5 Merge pull request #54 from GaelleLeroux/main
| |\ \  
| | * \   9d0d4c7 Merge branch 'main' into main
| | |\ \  
| | |/ /  
| |/| |   
| * | | 77ddc37 (tag: wsl2_windows) DOC: updated link to trained model CBCT
| * | | 69ef606 FIX: bounds checking and non-zero for dimension (CLI)
| * | | 1020d59 Update README.md - trained models ALI CBCT
| * | | fed158b (tag: v0.1-v2.0_models) Update README.md
| * | | 2105c88 BUG: Flybyprocess = autocrop3d_cli
| * | | b604e97 BUG : try to fix error load module
| * | |   7455a6e Merge pull request #47 from GaelleLeroux/main

With the same from #58

* 6b51b82 (upstream/rebased-autocrop-aso, rebased-autocrop-aso) BUG: fixed import module ASO and "__init__" in slicer
* c061d6a ENH: check box to keep the original siize of the file
*   8602659 (upstream/main, upstream/HEAD, main) Merge pull request #55 from GaelleLeroux/main
|\  
| * 9c9b47d PERF : resample using exportNode
......................................................
| * 7bdcb96 BUG : wrong interface cbct
|/  
*   e478693 Merge pull request #53 from GaelleLeroux/pytorch3d
|\  
| * df83de5 STYLE : delete old repository
......................................................
| * 230d1cd STYLE : add print test branch
* |   09462f5 Merge pull request #54 from GaelleLeroux/main
|\ \  
| * \   9d0d4c7 Merge branch 'main' into main
| |\ \  
| |/ /  
|/| |   
* | | 77ddc37 (tag: wsl2_windows) DOC: updated link to trained model CBCT
* | | 69ef606 FIX: bounds checking and non-zero for dimension (CLI)
* | | 1020d59 Update README.md - trained models ALI CBCT
* | | fed158b (tag: v0.1-v2.0_models) Update README.md
* | | 2105c88 BUG: Flybyprocess = autocrop3d_cli
* | | b604e97 BUG : try to fix error load module
* | |   7455a6e Merge pull request #47 from GaelleLeroux/main

Notice that commits from #53 like "BUG: Flybyprocess = autocrop3d_cli" are no longer duplicated, and notice that https://github.com/DCBIA-OrthoLab/SlicerAutomatedDentalTools/pull/58/commits only includes the two relevant changes.

There are still some strange merges with #53 and #54. I probably should have noticed and made these suggestions before merging those, but I didn't and now those commits are already in main. I think it best to bring everything back in sync now, and try to keep things linear moving forward.

Jeanneclre commented 6 months ago

Hello @allemangD

I think the best is to close the PR so I can do it better. Also I found out that something is not working in the commit BUG: fixed import module ASO and "__init__" in slicer so it should not be accepted.

Thank you!

allemangD commented 6 months ago

Closing per @Jeanneclre's comment. Will wait for a new rebased PR with the mentioned bug addressed.