Add distance summary plots #48
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #48 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 14 14
Lines 838 965 +127
==========================================
+ Hits 838 965 +127
|
kaitj
force-pushed
the
enh/plot-distance-summary
branch
from
1d13445
to
a001d53
Compare
kaitj
force-pushed
the
enh/plot-distance-summary
branch
from
a001d53
to
d882612
Compare
This commit adds the functionality to generate a summary plot of AFID distances on a connectome using nilearn.plotting to do so. In doing so, a complete set of AfidSet distances are provided as an argument, and coloured based on their distance from "expected" (ground-truth). Approporiate unit tests are also added in to make sure plots are generated and of the correct type. During development, generated plots were visually assessed to ensure generated plots are as expected. Note: a `pyright: ignore` has been temporarily added to line 230 in plotting.py until other plot_types are implemented in order to bypass the pyright error being thrown.
This commit adds the functionality to generate a summary histogram plot of AFID distances using `plotly`. In doing so, an optional argument has been added - `afid_labels` for labelling the histogram. If not provided, `afid_labels`, indices are used to label the histogram. Appropriate unit tests are also added. During development, generated plots were visually assessed. Note: `pyright: ignore` have been added where `afid_labels` is not expected to be None. Also added type stubs for plotly.
Knowing if the "distance" is negative can provide some insight - this is mostly for plotting distances along specific spatial components (e.g. x, y, z). These distances will be automatically set in the histogram and upcoming scatter plots. For the connectome plot, the minimum node value is set by either taking the smallest value (if negative) or 0 (if positive).
This commit adds functionality to generate a summary scatter plot of AFID distances using `plotly`. Similar to generating a histogram plot, this also makes use of the new `afid_labels` argument to label the markers. If not porvided, indices are used to label the scatter plot. Appropriate unit tests are alos added, with generated plots visually assessed during development.
Some tests may fail a HealthCheck due to slow data generation - namely any tests that try to generate an entire AfidSet. This commit adds a function to the helpers that disables this check.
This commit adds the slow_generation decorator where appropriate to disable the health checks for tests where data generation is slow. Additionally, delete the view that is created from plot_ortho for memory considerations (`close()` is not a valid command for these views).
kaitj
force-pushed
the
enh/plot-distance-summary
branch
from
d882612
to
7c8cc94
Compare
|
Also merging this one to push development. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
Based off of #45 (which should be merged in before this), this PR adds in functionality to create some distance-based summary plots. These include:
These all return views to allow the user to choose what they wish to do with it (e.g. save to disk, return html, etc.)
Also added typing stubs for
matplotlibandplotly(hence the 400k+ of new lines - these are all the files under thetypingsdirectory), which can be ignored.Types of changes
What types of changes does your code introduce? Put an
xin the boxes that applyChecklist
Put an
xin 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!poe qualitytaskNotes
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.