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

Revise Figure 5 code example #652

Merged
merged 4 commits into from
Mar 10, 2025
Merged

Conversation

dickscheid
Copy link
Contributor

@dickscheid dickscheid commented Mar 9, 2025

I rewrote the code example to make it closer to the manuscript figure and better understandable:

  • except for the initial raw voxel plot, all plots are done in physical coordinates using nilearn.plotting, removing all explicit point to voxel conversions. I achieved this by finding out how we can work with the 2D canvases of nilearn's 3D plot to plot contours, points, bounding boxes
  • The region of interest is not any more explicit, but defined just from a bounding box around a probability threshold of the pixels, so more intuitive now.
  • complicated filtering and aggregation of layer 4 vertex projections now much simplified. filtering removed since the plotting command filters implicitly.
  • overview plot with section position on glass brain now also implemented.

The PR should work, I tested several times, but please check it.

@dickscheid dickscheid requested a review from AhmetNSimsek March 9, 2025 08:50
Copy link

codecov bot commented Mar 9, 2025

Codecov Report

Attention: Patch coverage is 16.66667% with 5 lines in your changes missing coverage. Please review.

Project coverage is 47.99%. Comparing base (f891e91) to head (379bda2).
Report is 26 commits behind head on main.

Files with missing lines Patch % Lines
siibra/features/image/sections.py 16.66% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #652      +/-   ##
==========================================
+ Coverage   44.89%   47.99%   +3.10%     
==========================================
  Files          71       71              
  Lines        7311     7296      -15     
==========================================
+ Hits         3282     3502     +220     
+ Misses       4029     3794     -235     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@AhmetNSimsek AhmetNSimsek left a comment

Choose a reason for hiding this comment

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

Overall good but small changes could polish further. Please see my comments. (I can push these changes)

@@ -83,6 +83,14 @@ def __repr__(self):
def section(self) -> CellbodyStainedSection:
return self._section

def get_boundingbox(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

fetch_kwargs must be passed

Comment on lines 49 to 50
probmaps = region.parcellation.get_map("mni152", "statistical")
region_map = probmaps.get_volume(region)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not

region_map = region.get_regional_map("mni152", "statistical")  # this is a volume already

Comment on lines 87 to 90
patch_locations = siibra.PointCloud(
[tuple(p.get_boundingbox().center) for p in patches],
space='bigbrain'
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

patch_locations = siibra.PointCloud.union(*[p.get_boundingbox().center for p in patches])

@AhmetNSimsek AhmetNSimsek merged commit 0d6ba33 into main Mar 10, 2025
40 of 41 checks passed
@AhmetNSimsek AhmetNSimsek deleted the maint_streamline_fig5_example branch March 10, 2025 15:16
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.

2 participants