[ENH] Colorbar ticks at threshold values

[NicolasGensollen]

Fixes #2833

This PR proposes to display the threshold on the colorbar when plotting statistical maps.
The proposed solutions impacts only symmetrical colorbars and works with any odd number of ticks (5 was hardcoded prior to this PR).

Example

import nibabel as nib
import numpy as np
import matplotlib.pyplot as plt
from nilearn import plotting

# Create some fake data going from -10 to 10
img_data = np.zeros((10, 10, 10))
img_data[4:6, 2:4, 4:6] = -10
img_data[5:7, 3:7, 3:6] = 10
img = nib.Nifti1Image(img_data, affine=np.eye(4))

fig, axs = plt.subplots(4, 2)
fig.set_size_inches((15, 10))
for i,threshold in enumerate([0, 0.1, 1.3, 2.5, 4.9, 7.5, 8, 9.9]):
    plotting.plot_stat_map(img, threshold=threshold, 
                           title=f"threshold = {threshold}",
                           axes=axs[i//2,i%2])
plt.savefig("threshold.png")

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c16146a) 91.81% compared to head (91ea227) 91.83%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2887      +/-   ##
==========================================
+ Coverage   91.81%   91.83%   +0.01%     
==========================================
  Files         144      144              
  Lines       16173    16209      +36     
  Branches     3360     3373      +13     
==========================================
+ Hits        14850    14886      +36     
  Misses        781      781              
  Partials      542      542              

Flag	Coverage Δ
macos-latest_3.11_test_plotting	91.66% <100.00%> (?)
macos-latest_3.12_test_plotting	91.66% <100.00%> (?)
macos-latest_3.9_test_plotting	?
ubuntu-latest_3.10_test_plotting	91.66% <100.00%> (+0.01%)	⬆️
ubuntu-latest_3.11_test_plotting	91.66% <100.00%> (+0.01%)	⬆️
ubuntu-latest_3.12_test_plotting	91.66% <100.00%> (+0.01%)	⬆️
ubuntu-latest_3.12_test_pre	91.66% <100.00%> (+0.01%)	⬆️
ubuntu-latest_3.8_test_min	69.01% <0.00%> (-0.16%)	⬇️
ubuntu-latest_3.8_test_plot_min	91.38% <100.00%> (+0.01%)	⬆️
ubuntu-latest_3.8_test_plotting	91.62% <100.00%> (+0.01%)	⬆️
ubuntu-latest_3.9_test_plotting	91.63% <100.00%> (+0.01%)	⬆️
windows-latest_3.10_test_plotting	91.64% <100.00%> (+0.01%)	⬆️
windows-latest_3.11_test_plotting	91.64% <100.00%> (?)
windows-latest_3.12_test_plotting	91.64% <100.00%> (?)
windows-latest_3.8_test_plotting	91.60% <100.00%> (+0.01%)	⬆️
windows-latest_3.9_test_plotting	91.60% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[bthirion]

LGTM, thx !

• If we have an symmetric colorbar, don't we want the same behavior: on tick at the unique threshold value ?
• What about view_img ?

[jeromedockes]

* we have an symmetric colorbar, don't we want the same behavior: on tick at the unique threshold value ?

either that or removing the 0?

[NicolasGensollen]

If we have an symmetric colorbar, don't we want the same behavior: on tick at the unique threshold value ?

either that or removing the 0?

I implemented something alike which swaps the closest tick with the threshold value. So if the threshold is closer to zero, zero will be replaced by the threshold value:

Example:

import nibabel as nib
import numpy as np
import matplotlib.pyplot as plt
from nilearn import plotting

# Create some fake data going from 0 to 10
img_data = np.zeros((10, 10, 10))
img_data[4:6, 2:4, 4:6] = 5
img_data[5:7, 3:7, 3:6] = 10
img = nib.Nifti1Image(img_data, affine=np.eye(4))

fig, axs = plt.subplots(4, 2)
fig.set_size_inches((15, 10))
for i,threshold in enumerate([0, 0.1, 1.3, 2.5, 4.9, 7.5, 8, 9.9]):
    plotting.plot_stat_map(img, threshold=threshold, 
                           title=f"threshold = {threshold}",
                           axes=axs[i//2,i%2])
plt.savefig("out.jpeg")

What about view_img ?

Indeed, I need to look at other plotting functions with thresholded colorbars which don't rely on the BaseSlicer. I think plot_surf_stat_map for example should also have the same behavior for consistency.

[bthirion]

Thx. In the example above, the case with threshold=8 is weird: the colormap is clipped at 8 ?

[bthirion]

At least it looks good now.

Are the codecov warning serious ?

[bthirion]

LGTM, thx.

[jeromedockes]

@NicolasGensollen what is the status of this one? after having a quick look it looks like it could be merged before the next release?

[ymzayek]

It's not clear to me why this was never merged. Did it get blocked by something? is this behavior still desriable?

[Remi-Gau]

symmetric color bar

before

after

asymmetric color bar

before

after

[Remi-Gau]

To check: increase of warnings?

/home/remi/github/nilearn/temp.py:18: MatplotlibDeprecationWarning: savefig() got unexpected keyword argument "facecolor" which is no longer supported as of 3.3 and will become an error two minor releases later
  plt.savefig("threshold.png")
/home/remi/github/nilearn/temp.py:18: MatplotlibDeprecationWarning: savefig() got unexpected keyword argument "edgecolor" which is no longer supported as of 3.3 and will become an error two minor releases later
  plt.savefig("threshold.png")
/home/remi/github/nilearn/temp.py:18: MatplotlibDeprecationWarning: savefig() got unexpected keyword argument "orientation" which is no longer supported as of 3.3 and will become an error two minor releases later
  plt.savefig("threshold.png")
/home/remi/github/nilearn/temp.py:18: MatplotlibDeprecationWarning: savefig() got unexpected keyword argument "bbox_inches_restore" which is no longer supported as of 3.3 and will become an error two minor releases later
  plt.savefig("threshold.png")

[Remi-Gau]

TODO:

• check if this works with surfaces
• add tests with surfaces ?
• improve coverage
• check for an eventual increase in the number of warnings

[Remi-Gau]

@michellewang
I need to make sure the tests pass for this but I would not mind you having a review at this at some point because you have been changing code in those sections recently so I am sure you will spot things I am missing.

[michellewang]

@Remi-Gau

I did some (basic) manual tests with some of the surface plotting functions. The threshold ticks seem to work for plot_surf, plot_surf_roi, and plot_surf_stat_map. For plot_img_on_surf it doesn't work though:

from nilearn import datasets
from nilearn import plotting

stat_img = datasets.load_sample_motor_activation_image()

plotting.plot_img_on_surf(
    stat_img,
    views=['lateral', 'medial'],
    hemispheres=['left', 'right'],
    colorbar=True,
    vmin=-5,
    vmax=10,
    symmetric_cbar=False,
    threshold=2,
    cmap='viridis',
)

That's because in plot_img_on_surf the colorbar is created outside of the plot_surf_stat_map call. It doesn't take much to change the colorbar ticks here too, just something like:

def plot_img_on_surf(...
    cbar_tick_format='%i', # ADD new parameter with default tick format
    ...):

    ...

    # around line 1464
    if colorbar:
        sm = _colorbar_from_array(
            image.get_data(stat_map),
            vmin,
            vmax,
            threshold,
            symmetric_cbar=symmetric_cbar,
            cmap=plt.get_cmap(cmap),
        )

        cbar_grid = gridspec.GridSpecFromSubplotSpec(3, 3, grid[-1, :])
        cbar_ax = fig.add_subplot(cbar_grid[1])
        axes.append(cbar_ax)

        # ADD: get custom ticks
        ticks = _get_ticks_matplotlib(vmin, vmax, cbar_tick_format, threshold)

        # UPDATE: set ticks and format when making the colorbar
        fig.colorbar(sm, cax=cbar_ax, orientation='horizontal', ticks=ticks, format=cbar_tick_format)

    ...

Plot with modified code:

This branch doesn't include the changes I made in #3993 yet, but the new colorbar ticks work for plot_stat_map and plot_glass_brain, and I don't think anything should break after incorporating my changes 🤞

I also checked the test failures, I don't think they're directly related to anything I added, but I think I figured out why they failed.

FAILED nilearn/plotting/tests/test_surf_plotting.py::test_plot_surf_roi_colorbar_vmin_equal_across_engines[kwargs1] - AssertionError: assert -5 == 2.0
 +  where -5 = int('-5')
 +    where '-5' = <bound method Text.get_text of Text(1, -5.0, '-5')>()
 +      where <bound method Text.get_text of Text(1, -5.0, '-5')> = Text(1, -5.0, '-5').get_text

This is a weird one. Running the code manually shows that the two colorbars correspond to each other well (see below, left is Matplotlib, right is Plotly).

But the Matplotlib plot has the smallest ytick as -5 instead of 2. It seems like this is because, when there is a threshold , -threshold is added as a tick even if it is out of range for the colorbar (whose y-axis goes from 2 to 20 in this case).

This can be fixed by changing the test so that it uses something like mpl_plot.axes[-1].get_ylim()[0] instead of int(mpl_plot.axes[-1].get_yticklabels()[0].get_text()). I think that would make sense since otherwise the test assumes that the first tick is equal to vmin for the colorbar, which might not be the case (e.g. if the first tick is not on the edge of the colorbar). But we might also want to remove the extra (invisible) tick to avoid future confusion, maybe with something like this near the end of _get_cbar_ticks():

if len(ticks) > 0 and ticks[0] < vmin:
    ticks[0] = vmin

FAILED nilearn/plotting/tests/test_surf_plotting.py::test_plot_surf_roi_colorbar_vmin_equal_across_engines[kwargs2] - AssertionError: assert -5 == 0.0
 +  where -5 = int('-5')
 +    where '-5' = <bound method Text.get_text of Text(1, -5.0, '-5')>()
 +      where <bound method Text.get_text of Text(1, -5.0, '-5')> = Text(1, -5.0, '-5').get_text

^ This one failed for the same reason as the previous one.

FAILED nilearn/plotting/tests/test_surf_plotting.py::test_get_ticks_matplotlib[0-5e-324-%.1f-expected10] - assert (5 == 2)
 +  where 5 = len(array([0.e+000, 0.e+000, 0.e+000, 5.e-324, 5.e-324]))
 +  and   2 = len([0.0, 5e-324])

I think this one can be fixed by making sure that _get_cbar_ticks returns unique values (e.g. by wrapping the output with np.unique()).

[bthirion]

I think it's good for merging now.

[ymzayek]

Agreed