Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Example using the VSD database. #109

Open
wants to merge 31 commits into
base: VSD
Choose a base branch
from

Conversation

MCM-Fischer
Copy link

No description provided.

@modenaxe
Copy link
Owner

Thank you @MCM-Fischer - sorry if this is taking ages! @renaultJB so you have time to review the proposed fix to the STAPLE-Talus algorithm?

@renaultJB
Copy link
Collaborator

Thank you @MCM-Fischer and @modenaxe, I will review it this weekend 😃

@MCM-Fischer
Copy link
Author

@modenaxe No problem ;-).
Maybe it makes more sense if I push the talus fix separately to the main branch?

@modenaxe
Copy link
Owner

modenaxe commented Nov 19, 2022

@MCM-Fischer I think it would be indeed easier to manage and merge, please tag @renaultJB if you do so he's updated as well

@modenaxe
Copy link
Owner

modenaxe commented Jun 5, 2023

Hi @MCM-Fischer I am finally back to this. I have pushed to your repository a new proposed location for the example, in the msk-STAPLE\advanced_examples\process_VSD_dataset folder.
Comments/reasons:

  1. your script downloads the VSD dataset, that is relatively large compare to the size of STAPLE
  2. the script requires matGeom. Is it possible to donwload that from GitHub as you do for the VSD dataset?
  3. I would place both packages in the same folder, checking them out without git history git clone --depth 1 to minimise the download.

I think the example is great but I want to keep the dependencies and size of the main package minimal (both for users benefit and for our benefit in maintenance). @renaultJB do you agree?

@MCM-Fischer have you published the dataset paper as well?

Thank you,

Luca

@modenaxe
Copy link
Owner

modenaxe commented Jun 5, 2023

Also, @MCM-Fischer I have merged the main branch (with your talus fixes) to the VSD branch with the example but I still have issue processing some datasets, e.g. VSD_010_L. Could you please double check the code is properly integrated?

@MCM-Fischer
Copy link
Author

Also, @MCM-Fischer I have merged the main branch (with your talus fixes) to the VSD branch with the example but I still have issue processing some datasets, e.g. VSD_010_L. Could you please double check the code is properly integrated?

Should be fixed with Pull request #117

@MCM-Fischer
Copy link
Author

MCM-Fischer commented Nov 5, 2023

Hi @MCM-Fischer I am finally back to this. I have pushed to your repository a new proposed location for the example, in the msk-STAPLE\advanced_examples\process_VSD_dataset folder. Comments/reasons:

1. your script downloads the VSD dataset, that is relatively large compare to the size of STAPLE

2. the script requires [matGeom](https://github.com/mattools/matGeom). Is it possible to donwload that from GitHub as you do for the VSD dataset?

3. I would place both packages in the same folder, checking them out without git history `git clone --depth 1` to minimise the download.

I think the example is great but I want to keep the dependencies and size of the main package minimal (both for users benefit and for our benefit in maintenance). @renaultJB do you agree?

Hi Luca

I've kept the VSD example in the main folder because it would not fit to the description in the README.md of the advanced examples. It is actually a one-click (Run F5) example now. VSD and matGeom are downloaded. However, the git history of the two repositories is deleted afterwards. No additional dependencies are created. VSD and matGeom folders have been added to .gitignore.

Kind regards

@MCM-Fischer
Copy link
Author

@MCM-Fischer have you published the dataset paper as well?

Hi Luca

Yes, finally it was published now:
Fischer, M.C.M. Database of segmentations and surface models of bones of the entire lower body created from cadaver CT scans. Sci Data 10, 763 (2023). https://doi.org/10.1038/s41597-023-02669-z

Kind regards

@modenaxe
Copy link
Owner

Hi @MCM-Fischer thank you for contributing with this example! I should have time to review this hopefully before the holiday break - thank you for your patience!

@MCM-Fischer
Copy link
Author

Hi Luca
At first please merge #117.
Kind regards

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