FIX: marker size issue in plot_connectome #2185 #2186

robbisg · 2019-10-17T16:31:33Z

codecov · 2019-10-17T18:10:21Z

Codecov Report

Merging #2186 into master will increase coverage by <.01%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           master    #2186      +/-   ##
==========================================
+ Coverage   92.34%   92.35%   +<.01%     
==========================================
  Files         149      149              
  Lines       18867    18872       +5     
  Branches     2302     2300       -2     
==========================================
+ Hits        17423    17429       +6     
- Misses        924      925       +1     
+ Partials      520      518       -2

Impacted Files	Coverage Δ
nilearn/plotting/displays.py	`88.31% <33.33%> (-0.22%)`	⬇️
nilearn/datasets/tests/test_func.py	`99.41% <0%> (ø)`	⬆️
nilearn/plotting/tests/test_js_plotting_utils.py	`97.6% <0%> (+0.01%)`	⬆️
nilearn/datasets/func.py	`81.42% <0%> (+0.42%)`	⬆️

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 510e724...1cae5f1. Read the comment docs.

bthirion

LGTM. I know that it is hard if not impossible to test. You confirm that it solves #2185 ?

nilearn/plotting/displays.py

jeromedockes · 2019-10-17T19:31:29Z

LGTM. I know that it is hard if not impossible to test. You confirm that it solves #2185 ?

not really, the scattered points are a PathCollection in the matplotlib axis collections, and the PathCollection has a get_sizes() method

jeromedockes · 2019-10-17T19:39:34Z

LGTM. I know that it is hard if not impossible to test. You confirm that it solves #2185 ?

not really, the scattered points are a PathCollection in the matplotlib axis collections, and the PathCollection has a get_sizes() method

although I agree it may not be worth the trouble

robbisg

I edited as suggested!

nilearn/plotting/displays.py

jeromedockes · 2019-10-18T08:45:43Z

I edited as suggested!

thanks!

jeromedockes · 2019-10-18T10:48:27Z

Yes, I was thinking about that. In the docs of ```plot_connectome``` the ```marker_size``` is a defined as scalar or array_like, so I supposed that it is not a list. BTW I can edit it quickly.

that would be great. a list would be considered acceptable for "array-like"

List and numpy array accepted

nilearn/plotting/displays.py

Co-Authored-By: jeromedockes <jerome@dockes.org>

jeromedockes

LGTM, thanks a lot @robbisg !

robbisg · 2019-10-18T12:43:21Z

It was a pleasure @jeromedockes !! Thank all of you for developing this package!!

kchawla-pi · 2019-10-18T12:55:08Z

An entry in whats_new under Fixes section for 0.6.0a

…earn into fix_plot_glass_brain

kchawla-pi · 2019-10-18T14:04:26Z

THis is almost complete, we need to correct the Flake8 stylistic erro. It is the last entry if you click on details for TravisCI. I am reproducing the log here for convenience:

Running flake8 on the diff in the range 510e724..25d969f (6 commit(s)):

--------------------------------------------------------------------------------

nilearn/plotting/displays.py:417:1: W293 blank line contains whitespace

nilearn/plotting/displays.py:420:1: W293 blank line contains whitespace

This means that the blank lines indicated here contain spaces or tab characters. These should be removed.

robbisg · 2019-10-18T14:07:07Z

@kchawla-pi I hope now it's ok. If not, can you suggest me the edit? Thank you! ;)

kchawla-pi · 2019-10-24T18:13:23Z

Hey @robbisg ~~That wasn't what I had meant~~, but it's not a complicated change, though unfamiliar. I'll look at this tomorrow.

kchawla-pi · 2019-10-25T10:00:46Z

Hey yeah, in the next set of change you did most of the formatting fix. There's only one remaining.

In nilearn/plotting/displays.py:419:15 (line 419, position 15), that line is not indented with spaces that are multiple of 4. Fix that to satisfy Flake8.

             marker_size = np.asarray(marker_size)[relevant_coords]

The indentation is 14 spaces long, so make it a multiple of 4, while keeping it at the appropriate level.

In your commit message, explain why the change was made, for example "Fixed improper indentation" . Git can already tell us which files were changed and what changes were made to the code.

…nt_modify_fetch_dev * 'master' of https://github.com/nilearn/nilearn: Verbose doc building to ease tracking of progress & diagnose stalls (nilearn#2203) New conda env is created once conda path has been activated Conda environment is created for full-builds Refactor CircleCI config for reduced redundancy (nilearn#2204) Installation should fail on Python < 3.5 (nilearn#2198) [MRG] Add get data function (nilearn#2172) fix error when colorscale given boolean array (nilearn#2193) remove is_valid filter (nilearn#2169) Moved new entries to next release FIX: marker size issue in plot_connectome nilearn#2185 (nilearn#2186) Updated Appveyor status badge Run tests on local Windows machines & Azure Pipelines (nilearn#2191) Updated requirements list for devs (nilearn#2190) Update azure-pipelines.yml for Azure Pipelines 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)

…smooth-image * 'master' of https://github.com/nilearn/nilearn: (25 commits) Add testing for Nilearn setup & install & Fix the broken dependency installation (nilearn#2201) Fix uniform ball cloud test for sklearn >= 0.22 (nilearn#2175) MAINT: sklearn Deprecations (nilearn#2219) DOC: title in bold [DOC] Add note about decreasing memory usage (nilearn#2223) Rel 060b0 (nilearn#2208) Nilearn 0.6.0b0 release (nilearn#2206) Fixed the redundant & missing test case in merged PR nilearn#2035 (nilearn#2205) Modify fetch_development_fmri to fetch adults or children (nilearn#2035) Verbose doc building to ease tracking of progress & diagnose stalls (nilearn#2203) New conda env is created once conda path has been activated Conda environment is created for full-builds Refactor CircleCI config for reduced redundancy (nilearn#2204) Installation should fail on Python < 3.5 (nilearn#2198) [MRG] Add get data function (nilearn#2172) fix error when colorscale given boolean array (nilearn#2193) remove is_valid filter (nilearn#2169) Moved new entries to next release FIX: marker size issue in plot_connectome nilearn#2185 (nilearn#2186) Updated Appveyor status badge ...

FIX: marker size issue in plot_connectome #2185

ebac7be

bthirion reviewed Oct 17, 2019

View reviewed changes

jeromedockes reviewed Oct 17, 2019

View reviewed changes

nilearn/plotting/displays.py Outdated Show resolved Hide resolved

Update displays.py

5b910f5

robbisg commented Oct 18, 2019

View reviewed changes

jeromedockes reviewed Oct 18, 2019

View reviewed changes

nilearn/plotting/displays.py Outdated Show resolved Hide resolved

Update displays.py

36f3f5b

List and numpy array accepted

jeromedockes reviewed Oct 18, 2019

View reviewed changes

nilearn/plotting/displays.py Outdated Show resolved Hide resolved

Update nilearn/plotting/displays.py

3bf419e

Co-Authored-By: jeromedockes <jerome@dockes.org>

jeromedockes approved these changes Oct 18, 2019

View reviewed changes

robbisg added 2 commits October 18, 2019 15:21

FIX: marker size issue in plot_connectome #2185

26ef753

Merge branch 'fix_plot_glass_brain' of https://github.com/robbisg/nil…

25d969f

…earn into fix_plot_glass_brain

Update whats_new.rst

97a874c

Update displays.py

a782cce

Fixed identation for flake

1cae5f1

kchawla-pi merged commit 7ca7d11 into nilearn:master Oct 28, 2019

robbisg deleted the fix_plot_glass_brain branch October 28, 2019 10:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FIX: marker size issue in plot_connectome #2185 #2186

FIX: marker size issue in plot_connectome #2185 #2186

robbisg commented Oct 17, 2019 •

edited by jeromedockes

codecov bot commented Oct 17, 2019 •

edited

bthirion left a comment

jeromedockes commented Oct 17, 2019

jeromedockes commented Oct 17, 2019

robbisg left a comment

jeromedockes commented Oct 18, 2019

jeromedockes commented Oct 18, 2019 via email

jeromedockes left a comment

robbisg commented Oct 18, 2019

kchawla-pi commented Oct 18, 2019

kchawla-pi commented Oct 18, 2019

robbisg commented Oct 18, 2019

kchawla-pi commented Oct 24, 2019 •

edited

kchawla-pi commented Oct 25, 2019 •

edited

FIX: marker size issue in plot_connectome #2185 #2186

FIX: marker size issue in plot_connectome #2185 #2186

Conversation

robbisg commented Oct 17, 2019 • edited by jeromedockes

codecov bot commented Oct 17, 2019 • edited

Codecov Report

bthirion left a comment

Choose a reason for hiding this comment

jeromedockes commented Oct 17, 2019

jeromedockes commented Oct 17, 2019

robbisg left a comment

Choose a reason for hiding this comment

jeromedockes commented Oct 18, 2019

jeromedockes commented Oct 18, 2019 via email

jeromedockes left a comment

Choose a reason for hiding this comment

robbisg commented Oct 18, 2019

kchawla-pi commented Oct 18, 2019

kchawla-pi commented Oct 18, 2019

robbisg commented Oct 18, 2019

kchawla-pi commented Oct 24, 2019 • edited

kchawla-pi commented Oct 25, 2019 • edited

robbisg commented Oct 17, 2019 •

edited by jeromedockes

codecov bot commented Oct 17, 2019 •

edited

kchawla-pi commented Oct 24, 2019 •

edited

kchawla-pi commented Oct 25, 2019 •

edited