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 orthoview AFID plot #45

Merged
merged 9 commits into from
Dec 6, 2023
Merged

Add orthoview AFID plot #45

merged 9 commits into from
Dec 6, 2023

Conversation

kaitj
Copy link
Contributor

@kaitj kaitj commented Oct 19, 2023

Proposed changes
This PR adds some plotting functionality to visualize the AFIDs provided by returning a view object with N number of AFIDs (up to 32) overlaid on a nifti image. The one caveat is that AFIDs are assumed to be in the same space as the provided image.

This plotting functionality is also setup as an extra dependency - that is by default, installing afids-utils will not provide plotting capabilities. To do so, one would have to specify it (e.g. using pip - pip install afids-utils[plotting]). This choice was made to avoid installation of extra libraries if the end-user does not intend to use it.

Types of changes
What types of changes does your code introduce? Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other (if none of the other choices apply)

Checklist
Put an x in the boxes that apply. You can also fill these out after creating the PR. If you are unsure about any of the choices, don't hesitate to ask!

  • Changes have been tested to ensure that fix is effective or that a feature works.
  • Changes pass the unit tests
  • Code has been run through the poe quality task
  • I have included necessary documentation or comments (as necessary)
  • Any dependent changes have been merged and published

Notes
All PRs will undergo the unit testing before being reviewed. You may be requested to explain or make additional changes before the PR is accepted.

@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #45 (8c998f7) into main (a8dfe09) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #45   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           12        14    +2     
  Lines          760       838   +78     
=========================================
+ Hits           760       838   +78     
Components Coverage Δ
afids_utils/afids.py 100.00% <ø> (ø)
afids_utils/ext 100.00% <ø> (ø)
afids_utils/metrics.py 100.00% <ø> (ø)
afids_utils/plotting.py 100.00% <100.00%> (∅)
afids_utils/transforms.py 100.00% <ø> (ø)

@kaitj kaitj marked this pull request as ready for review October 19, 2023 17:44
@kaitj kaitj self-assigned this Oct 19, 2023
@kaitj kaitj added the enhancement New feature or request label Oct 19, 2023
@kaitj kaitj requested a review from ataha24 October 19, 2023 17:45
@kaitj
Copy link
Contributor Author

kaitj commented Oct 19, 2023

@ataha24 - Added you to review this one since you may have some thoughts about additional features / changes regarding the plotting.

@kaitj kaitj force-pushed the enh/plotting branch 4 times, most recently from a3ae407 to 9b1eb1f Compare October 24, 2023 16:59
@kaitj kaitj force-pushed the enh/plotting branch 2 times, most recently from f73b776 to fa51c70 Compare October 28, 2023 20:55
@kaitj kaitj mentioned this pull request Oct 30, 2023
9 tasks
@kaitj kaitj force-pushed the enh/plotting branch 2 times, most recently from 2da9def to 5b339db Compare November 13, 2023 19:50
@kaitj
Copy link
Contributor Author

kaitj commented Dec 6, 2023

I am merging this in to push development forward.

- Add nilearn for orthogonal plotting, with matplotlib installation
- Separately install plotly (nilearn's plotly version requires another
  library that does not appear to support py3.10+)
- By adding this through extras instead of group dependencies, this
  should enable the ability to install using `pip install
afids-utils[plotting]`
This commit adds the functionality to create a view object with AFIDs
overlaid on an nifti image. To do so, a nifti image with just the
fiducials are created with an intensity value equal to the labels. This
serves as a "segmentation" or "label" map. A custom 32-discrete colormap
object is created to visualize these different fiducials. Additionally,
to allow for either voxel coordinates or positional coordinates to be
passed, a conversion to voxel coordinates is performed when the plotting
function is called. Note, plotting is not limited to complete `AfidSet`s.
Individual and incomplete `AfidSet`s can also be plotted.

Finally stubs have been created for `matplotlib` and `nilearn` to
correct for pyright errors. `# pyright: ignore` was added where
appropriate to bypass issues false errors.
Also fix type to afids_utils/afids.py
Issue with matplotlib installation that prevented the plotting page to
be loaded. This was due to a `poetry` installation command.

Also added intersphinx mappings for plotting functionality and made some
text changes to docstrings.
@kaitj kaitj merged commit b7bf221 into afids:main Dec 6, 2023
24 checks passed
@kaitj kaitj deleted the enh/plotting branch December 6, 2023 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant