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

Add support for spinal rootlets segmentation #98

Merged
merged 10 commits into from
Jul 18, 2024

Conversation

KaterinaKrejci231054
Copy link
Contributor

@KaterinaKrejci231054 KaterinaKrejci231054 commented Jun 27, 2024

This draft PR adds an option to correct rootlets segmentation.

Resolves: #56

manual_correction.py Outdated Show resolved Hide resolved
@valosekj
Copy link
Member

You can also add rootlets into this list: https:/spinalcordtoolbox/manual-correction/blob/main/README.md?plain=1#L5-L12

@valosekj
Copy link
Member

valosekj commented Jun 29, 2024

I added some unit tests for recently implemented get_orientation and change_orientation functions (#100) in cde0916.
The tests worked fine locally. But when triggered using the GitHub action, the tests failed on missing sct_image (which is used by get_orientation and change_orientation, e.g., here).

image

sct_image is missing because we do not currently install SCT for our CI. Not sure if we want to install SCT for our CI as it would increase the CI time a lot...

Copy link
Member

@valosekj valosekj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, @KaterinaKrejci231054! I tested the PR using the following command, and it worked perfectly!

python manual_correction.py -path-img ~/data/results/philadelphia-pediatric/philadelphia-pediatric_2024-06-26/data_processed -path-label ~/data/results/philadelphia-pediatric/philadelphia-pediatric_2024-06-26/derivatives/labels -change-orient RPI -path-out ~/data/results/philadelphia-pediatric/philadelphia-pediatric_2024-06-26/derivatives_corr/labels -config ~/data/results/philadelphia-pediatric/philadelphia-pediatric_2024-06-26/three_subjects_to_correct.json -fsleyes-cm jet_matplotlib

I'm pre-approving the PR.
Can you please address this comment?
I'll address the failing CI. Then, we can merge!

@valosekj
Copy link
Member

valosekj commented Jul 16, 2024

Hey, @joshuacwnewton! I added SCT installation into the manual_correction CI in d58dfee and 56b375f, but CI still struggles to locate sct_image:

...
/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/subprocess.py:1720: FileNotFoundError
=========================== short test summary info ============================
FAILED tests/test_utils.py::test_get_orientation - FileNotFoundError: [Errno 2] No such file or directory: 'sct_image'
FAILED tests/test_utils.py::test_change_orientation - FileNotFoundError: [Errno 2] No such file or directory: 'sct_image'
========================= 2 failed, 9 passed in 0.40s ==========================

Do you have any ideas on how to solve this?

@joshuacwnewton
Copy link
Member

Thanks for the ping! I have a bit of a backlog to get through since coming back from the conference I was attending, but I'm very happy to take a look at this soon. :)

@joshuacwnewton joshuacwnewton removed their request for review July 18, 2024 15:40
@valosekj valosekj marked this pull request as ready for review July 18, 2024 15:49
@valosekj
Copy link
Member

@KaterinaKrejci231054, thanks for this PR, and @joshuacwnewton, thanks for fixing the CI!
LGTM now! merging

@valosekj valosekj merged commit 4a23c14 into main Jul 18, 2024
1 check passed
valosekj added a commit that referenced this pull request Jul 18, 2024
@valosekj valosekj deleted the kk/56_add_rootlets_seg_editing branch July 18, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for spinal rootlets segmentation
3 participants