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

[WIP] ENH: add connectome strength plot #2028

Merged
merged 11 commits into from
Oct 14, 2019

Conversation

glemaitre
Copy link
Contributor

@glemaitre glemaitre commented Apr 18, 2019

This PR adds the possibility to plot the strength of a connectome on a glass brain.
The strength is defined as the absolute sum of the edges at a node.

TODO:

  • add example for visualization
  • add tests

@codecov
Copy link

codecov bot commented Apr 18, 2019

Codecov Report

Merging #2028 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2028      +/-   ##
==========================================
+ Coverage   92.31%   92.34%   +0.03%     
==========================================
  Files         149      149              
  Lines       18781    18867      +86     
  Branches     2290     2302      +12     
==========================================
+ Hits        17337    17423      +86     
- Misses        922      924       +2     
+ Partials      522      520       -2
Impacted Files Coverage Δ
nilearn/plotting/__init__.py 95.65% <ø> (ø) ⬆️
nilearn/plotting/tests/test_img_plotting.py 99.82% <100%> (+0.01%) ⬆️
nilearn/plotting/img_plotting.py 89.78% <100%> (+1.51%) ⬆️
nilearn/datasets/neurovault.py 88.43% <0%> (-0.25%) ⬇️
nilearn/plotting/tests/test_html_connectome.py 100% <0%> (ø) ⬆️
nilearn/tests/test_cache_mixin.py 95.65% <0%> (+1.29%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c24005...54edbcb. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 18, 2019

Codecov Report

Merging #2028 into master will decrease coverage by 0.09%.
The diff coverage is 5.26%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #2028     +/-   ##
=========================================
- Coverage   94.96%   94.86%   -0.1%     
=========================================
  Files         138      138             
  Lines       17878    17897     +19     
=========================================
+ Hits        16977    16978      +1     
- Misses        901      919     +18
Impacted Files Coverage Δ
nilearn/plotting/__init__.py 100% <ø> (ø) ⬆️
nilearn/plotting/img_plotting.py 88.59% <5.26%> (-5.5%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95fb16e...7dc05ff. Read the comment docs.

@GaelVaroquaux
Copy link
Member

Sorry, we've merge @illdopejake 's PR in master. There are now conflicts to address :(.

@glemaitre
Copy link
Contributor Author

I'll fix that

@illdopejake
Copy link
Contributor

Let me know if I can help

@glemaitre
Copy link
Contributor Author

@illdopejake Should be good to be reviewed.

@illdopejake
Copy link
Contributor

illdopejake commented Apr 19, 2019

@glemaitre Looks great!

The one thing that we are going to explore, however, is whether the crappy connectomes that we are seeing are because the first several subjects are young children, rather than adults.

I will have a look at this and maybe we can set an argument to the fetcher to grab children or adults specifically. That way we, maybe we won't need to download 20 subjects just to run the example.

@illdopejake
Copy link
Contributor

fyi just added the PR to a) get adults first, b) choose if you want children or adults. Find it here: #2035

display.close()
display = None

return display
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey @glemaitre this is a super long method, refactoring it would be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But there is no code duplication. It would not be really meaningful to cut the function into smaller functions if they are not reused.

Copy link
Collaborator

Choose a reason for hiding this comment

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

They improve ease of understanding.

@kchawla-pi
Copy link
Collaborator

Add a whats_new entry, pull from master.

@GaelVaroquaux
Copy link
Member

GaelVaroquaux commented Aug 2, 2019 via email


adjacency_matrix = np.nan_to_num(adjacency_matrix)

adjacency_matrix_shape = adjacency_matrix.shape
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the 4 checks below (adjacency shape, nodes shape, both shapes, adjacency symmetry, mask symmetry) could be useful for other connectome plotting functions?

@kchawla-pi
Copy link
Collaborator

@glemaitre We are aiming for an end of September release. Think this can be included?

@jeromedockes
Copy link
Member

I think this can be merged once rebased. My comment about turning the checks into functions can be ignored or left for a later PR. Hence @glemaitre do you mind rebasing?

…lot_connectome_strength

* 'master' of https://github.com/nilearn/nilearn: (272 commits)
  [MRG] FIX: orientation problem with plot_glass_brain (nilearn#1888)
  Import HTMLDocument in its original module to preserve backwards compatibility (nilearn#2162)
  Fixing malfunctioning allowed-failure section in Travis (nilearn#2160)
  Update Brainomics fetcher (nilearn#2097)
  Remove inplace modification in signal.clean (nilearn#2125)
  Iter age group prediction example (nilearn#2063)
  Expose bg_img, vmin and vmax in plot_img signature (nilearn#2157)
  Replac conda with pip in TravisCI setup (nilearn#2141)
  Core devs doc and add @emdupre (nilearn#2151)
  Add check for vmin, vmax in plot_surf_roi (nilearn#2052)
  [ENH] Initial visual reports (nilearn#2019)
  Renamed test to deduplicate name (nilearn#2144)
  Fixes nilearn#2029 Handle gzip files without extensions (nilearn#2126)
  Update Schaefer parcelation to v0.14.3 (nilearn#2138)
  MAINT: Future-compatible cmap reversal (nilearn#2131)
  fix openmp crash (nilearn#2140)
  change nose to pytest on appveyor (nilearn#2130)
  adding fix to whatsnew
  fix wrong urls in nki dataset
  Made Flake8 happy
  ...

# Conflicts:
#	examples/03_connectivity/plot_sphere_based_connectome.py
@kchawla-pi
Copy link
Collaborator

CircleCI error:

File "/home/circleci/miniconda3/envs/testenv/lib/python3.6/site-packages/sklearn/covariance/graph_lasso_.py", line 247, in graphical_lasso
    raise FloatingPointError('The system is too ill-conditioned '
FloatingPointError: The system is too ill-conditioned for this solver. The system is too ill-conditioned for this solver

@kchawla-pi
Copy link
Collaborator

Before the OSF problem I was seeing this error. We must not merge this until there is a reliable solution to this problem.

@kchawla-pi kchawla-pi changed the title EHN: add connectome strength plot [WIP] EHN: add connectome strength plot Oct 13, 2019
@kchawla-pi kchawla-pi changed the title [WIP] EHN: add connectome strength plot [WIP] ENH: add connectome strength plot Oct 13, 2019
@bthirion
Copy link
Member

Before the OSF problem I was seeing this error. We must not merge this until there is a reliable solution to this problem.

This is a numerical proble; hence, to move forward on this, we need to localize the problem: with which dataset does it occur ? What is the problem with this dataset.
I have tried to check, but the CircleCI failure I can see is related to download...

@kchawla-pi
Copy link
Collaborator

@jeromedockes
Copy link
Member

jeromedockes commented Oct 13, 2019 via email

@jeromedockes
Copy link
Member

note that the link above is from another pr:
#2035

but the error can be reproduced with either pr

@jeromedockes
Copy link
Member

I open a PR on @glemaitre 's branch that fixes the issue for this one:

glemaitre#1

@kchawla-pi
Copy link
Collaborator

@jeromedockes Anything you wanna change? Or imma merge this once green.

@jeromedockes
Copy link
Member

you can merge. I think we should remove the last atlas from the plot_spheres example but it can be done in a separate pr

@kchawla-pi
Copy link
Collaborator

I will remove it now, and if we decide otherwise, we can add it back in another PR.

@jeromedockes
Copy link
Member

jeromedockes commented Oct 14, 2019 via email

@kchawla-pi
Copy link
Collaborator

Gah right. No worries.

@kchawla-pi kchawla-pi merged commit b5726d6 into nilearn:master Oct 14, 2019
kchawla-pi added a commit to illdopejake/nilearn that referenced this pull request Oct 14, 2019
…nt_modify_fetch_dev

* 'master' of https://github.com/nilearn/nilearn:
  ENH: add connectome strength plot (nilearn#2028)
  Fix cache mixin tests (nilearn#2161)
  Do not fail if metadata cannot be updated for an image (nilearn#2167)
kchawla-pi added a commit to kchawla-pi/nilearn that referenced this pull request Oct 14, 2019
kchawla-pi added a commit that referenced this pull request Oct 14, 2019
…trength) (#2174)

* Fix Flake8 errors overlooked when merging PR #2028 (Plot connectome strength)

* Ignore linting for W508

* Corrected linter code
kchawla-pi added a commit to kchawla-pi/nilearn that referenced this pull request Oct 14, 2019
* 'master' of github.com:nilearn/nilearn:
  Fix Flake8 errors overlooked when merging PR nilearn#2028 (Plot connectome strength) (nilearn#2174)
  ENH: add connectome strength plot (nilearn#2028)
kchawla-pi added a commit to KamalakerDadi/nilearn that referenced this pull request Oct 14, 2019
…ecomposition

* 'master' of https://github.com/nilearn/nilearn:
  Fix Flake8 errors overlooked when merging PR nilearn#2028 (Plot connectome strength) (nilearn#2174)
  ENH: add connectome strength plot (nilearn#2028)
  Fix cache mixin tests (nilearn#2161)
  Do not fail if metadata cannot be updated for an image (nilearn#2167)
  [MRG] Nans in view connectome (nilearn#2166)
kchawla-pi added a commit to illdopejake/nilearn that referenced this pull request Oct 17, 2019
…nt_modify_fetch_dev

* 'master' of https://github.com/nilearn/nilearn:
  Removed sub-example due to unfit for lasso dataset - unstable float values (nilearn#2177)
  Fix Flake8 errors overlooked when merging PR nilearn#2028 (Plot connectome strength) (nilearn#2174)
kchawla-pi added a commit to pausz/nilearn that referenced this pull request Oct 18, 2019
…smooth-image

* 'master' of https://github.com/nilearn/nilearn: (114 commits)
  [DOC] Update whats_new to reference nilearn#2013 (Merging of several examples) (nilearn#2183)
  Removed sub-example due to unfit for lasso dataset - unstable float values (nilearn#2177)
  Fix Flake8 errors overlooked when merging PR nilearn#2028 (Plot connectome strength) (nilearn#2174)
  ENH: add connectome strength plot (nilearn#2028)
  Fix cache mixin tests (nilearn#2161)
  Do not fail if metadata cannot be updated for an image (nilearn#2167)
  [MRG] Nans in view connectome (nilearn#2166)
  [MRG] FIX: orientation problem with plot_glass_brain (nilearn#1888)
  Import HTMLDocument in its original module to preserve backwards compatibility (nilearn#2162)
  Fixing malfunctioning allowed-failure section in Travis (nilearn#2160)
  Update Brainomics fetcher (nilearn#2097)
  Remove inplace modification in signal.clean (nilearn#2125)
  Iter age group prediction example (nilearn#2063)
  Expose bg_img, vmin and vmax in plot_img signature (nilearn#2157)
  Replac conda with pip in TravisCI setup (nilearn#2141)
  Core devs doc and add @emdupre (nilearn#2151)
  Add check for vmin, vmax in plot_surf_roi (nilearn#2052)
  [ENH] Initial visual reports (nilearn#2019)
  Renamed test to deduplicate name (nilearn#2144)
  Fixes nilearn#2029 Handle gzip files without extensions (nilearn#2126)
  ...
kchawla-pi added a commit to kchawla-pi/nilearn that referenced this pull request Oct 24, 2019
…te-req-dev

* 'master' of https://github.com/nilearn/nilearn: (336 commits)
  Release Nilearn 0.6.0 alpha (nilearn#2164)
  Making fetch_localizer_button_task backwards compatibile (nilearn#2182)
  [DOC] Update whats_new to reference nilearn#2013 (Merging of several examples) (nilearn#2183)
  Removed sub-example due to unfit for lasso dataset - unstable float values (nilearn#2177)
  Fix Flake8 errors overlooked when merging PR nilearn#2028 (Plot connectome strength) (nilearn#2174)
  ENH: add connectome strength plot (nilearn#2028)
  Fix cache mixin tests (nilearn#2161)
  Do not fail if metadata cannot be updated for an image (nilearn#2167)
  [MRG] Nans in view connectome (nilearn#2166)
  [MRG] FIX: orientation problem with plot_glass_brain (nilearn#1888)
  Import HTMLDocument in its original module to preserve backwards compatibility (nilearn#2162)
  Fixing malfunctioning allowed-failure section in Travis (nilearn#2160)
  Update Brainomics fetcher (nilearn#2097)
  Remove inplace modification in signal.clean (nilearn#2125)
  Iter age group prediction example (nilearn#2063)
  Expose bg_img, vmin and vmax in plot_img signature (nilearn#2157)
  Replac conda with pip in TravisCI setup (nilearn#2141)
  Core devs doc and add @emdupre (nilearn#2151)
  Add check for vmin, vmax in plot_surf_roi (nilearn#2052)
  [ENH] Initial visual reports (nilearn#2019)
  ...
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.

None yet

6 participants