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

Use nearest-neighbors interpolation in plot_component #1098

Merged
merged 2 commits into from
May 22, 2024

Conversation

tsalo
Copy link
Member

@tsalo tsalo commented May 21, 2024

Closes #1095.

Changes proposed in this pull request:

  • Switch from the default 'continuous' resampling approach to 'nearest' in plot_component's plot_stat_map call. This should stop non-diagonal images from being resampled to have low, non-zero values outside of the brain.

@tsalo tsalo added bug issues describing a bug or error found in the project reports issues related to boilerplate generation or visual reports labels May 21, 2024
@tsalo
Copy link
Member Author

tsalo commented May 21, 2024

From Logan's 8-echo dataset.

Before:

image

After:

image

@tsalo tsalo marked this pull request as ready for review May 21, 2024 21:34
@tsalo tsalo requested a review from handwerkerd May 21, 2024 21:34
Copy link

codecov bot commented May 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.91%. Comparing base (12aea7f) to head (d8df3e6).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1098   +/-   ##
=======================================
  Coverage   89.91%   89.91%           
=======================================
  Files          26       26           
  Lines        3621     3621           
  Branches      629      629           
=======================================
  Hits         3256     3256           
  Misses        214      214           
  Partials      151      151           

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

Copy link
Member

@handwerkerd handwerkerd left a comment

Choose a reason for hiding this comment

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

For the three times vmin is set to the 2nd percentile, you would not need to set resampling_interpolation="nearest" and "continuous" will result in nicer looking images.
For the one situation where vmin isn't set, setting vmin=np.min(stat_img.get_fdata()) might give slightly nicer visualizations, but setting interpolation to "nearest" is also fine.

@tsalo tsalo changed the title Use nearest-neighbors interpolation in plot_stat_map calls Use nearest-neighbors interpolation in plot_component May 22, 2024
@tsalo
Copy link
Member Author

tsalo commented May 22, 2024

Good point. I dropped the change for plot_stat_map calls outside of plot_component.

Copy link
Member

@handwerkerd handwerkerd left a comment

Choose a reason for hiding this comment

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

LGTM

I also realized my suggestion for adding vmin to stat_map figures is a bit more complex, since there are positive & negative values. It is solvable, but this solution is fine.

@tsalo
Copy link
Member Author

tsalo commented May 22, 2024

Yeah we'd need to use threshold instead of vmin, but I played around with that and had some issues, so I think this is fine for now. If we end up overlaying our stat maps on other images (e.g., mean OC, T1w) then we might need to revisit.

@tsalo tsalo merged commit 2e9831b into ME-ICA:main May 22, 2024
16 checks passed
@tsalo tsalo deleted the fix-plotting branch May 22, 2024 14:39
@tsalo
Copy link
Member Author

tsalo commented May 22, 2024

I'd like to make a release with this change before we merging #1064. @handwerkerd any objections?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug issues describing a bug or error found in the project reports issues related to boilerplate generation or visual reports
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display of IC components has a grey outline in report
2 participants