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 tutorial for sct_compute_compression in documentation #4162

Merged
merged 96 commits into from Oct 11, 2023

Conversation

sandrinebedard
Copy link
Member

@sandrinebedard sandrinebedard commented Jul 19, 2023

Checklist

GitHub

PR contents

Description

This PR adresses #4158. It adds a tutorial for sct_compute_compression

It includes:

  1. Before starting
  2. Theory section
  3. Generating input files
  4. sct_compute_compression usage

Linked issues

Resolves #4158

Data

Links are not working yet for the data, so here are the zip files:

data_compression.zip
data_normalizing-morphometrics-compression.zip

TODO

  • Add data to docs repository
  • Find another compression image to use as an example
  • Add images in the tutorial data
  • Add zip with input files if they want to skip

jcohenadad

This comment was marked as resolved.

jcohenadad

This comment was marked as resolved.

@sandrinebedard sandrinebedard added sct_compute_compression context: documentation category: readthedocs, sourceforge, or SCT courses labels Jul 19, 2023
@sandrinebedard sandrinebedard self-assigned this Jul 19, 2023
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.

Thanks for the updates, @sandrinebedard! The tutorial is excellent! I left only a few very minor suggestions.

sandrinebedard and others added 9 commits October 4, 2023 10:29
…ify-correctness.rst

Co-authored-by: Julien Cohen-Adad <jcohen@polymtl.ca>
…malizing-morphometrics-compressions/normalize-morphometrics.rst

Co-authored-by: Jan Valosek <39456460+valosekj@users.noreply.github.com>
…malizing-morphometrics-compressions/before-starting.rst

Co-authored-by: Jan Valosek <39456460+valosekj@users.noreply.github.com>
…malizing-morphometrics-compressions/normalize-morphometrics.rst

Co-authored-by: Jan Valosek <39456460+valosekj@users.noreply.github.com>
…malizing-morphometrics-compressions/normalize-morphometrics.rst

Co-authored-by: Jan Valosek <39456460+valosekj@users.noreply.github.com>
@sandrinebedard
Copy link
Member Author

@joshuacwnewton is there a way here: https://spinalcordtoolbox--4162.org.readthedocs.build/user_section/tutorials/shape-analysis.html to only have the Cross-sectional area (CSA) in the TOC and not the other CSA (since they are now in the same page?

@joshuacwnewton
Copy link
Member

joshuacwnewton commented Oct 6, 2023

Yes, of course!

When we include a page in the TOC, like this:

It will include links to all top-level headers in the file. And, in this case, the CSA page has multiple top-level headers (####):

Cross-sectional area (CSA)
######################################################
This section demonstrates how to compute spinal cord cross-sectional area.
CSA (Averaged across vertebral levels)
######################################

So, it is just a matter of bumping the lower headers down to the next level by using a symbol other than #.

This will make sure the sub-headers don't get included in the TOC
Copy link
Member

Choose a reason for hiding this comment

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

Hmm. Since the "Shape metrics" section has been reorganized, I realize the structure is a bit strange?

Shape analysis
-- Before starting
-- CSA
-- Other shape metrics
-- Verifying correctness
-- Compression
  -- Before starting
  -- [...]

There's kind of... two nested "before starting" pages? (It's like the first set of pages are individual pages, while "Compression" is a whole extra sub-tutorial.) Which could get a little confusing...

So, I'm wondering if a structure like this could be better:

Shape analysis
-- 1. Computing CSA, area, and other shape metrics
  -- Before starting
  -- CSA
  -- Other shape metrics
  -- Verifying correctness
-- 2. Compression
  -- Before starting
  -- [...]

That way, "Before starting" applies to each of the 2 different sub-tutorials (1. basic sct_process_segmentation usage 2. using sct_process_segmentation for compression analysis).

Copy link
Member

Choose a reason for hiding this comment

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

This is the same structure that the "Registration" section has -- one "main page" divided into separate self-contained sub-tutorials.

Copy link
Member

Choose a reason for hiding this comment

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

So, I'm wondering if a structure like this could be better:

Shape analysis
-- 1. Computing CSA, area, and other shape metrics
  -- Before starting
  -- CSA
  -- Other shape metrics
  -- Verifying correctness
-- 2. Compression
  -- Before starting
  -- [...]

Makes sense to me!

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense to me too!

Copy link
Member

Choose a reason for hiding this comment

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

Great! I'll make this change, check over any remaining outstanding feedback (i.e. the open comments from Julien at the start of the PR), and then merge this PR. :)

Copy link
Member

Choose a reason for hiding this comment

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

Done in da38c92!

I think we're good to merge now. :)

@joshuacwnewton joshuacwnewton mentioned this pull request Oct 10, 2023
16 tasks
@joshuacwnewton joshuacwnewton merged commit 78d46e0 into master Oct 11, 2023
24 checks passed
@joshuacwnewton joshuacwnewton deleted the sb/4158-add-tutorial-sct-compute-compression branch October 11, 2023 16:47
@joshuacwnewton joshuacwnewton added this to the 6.1 milestone Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation category: readthedocs, sourceforge, or SCT courses sct_compute_compression context:
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tutorial for sct_compute_compression function
5 participants