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

[BUG] plot_roi cannot handle cmap with only 1 level #4255

Closed
1 of 9 tasks
adelavega opened this issue Feb 2, 2024 · 10 comments · Fixed by #4256
Closed
1 of 9 tasks

[BUG] plot_roi cannot handle cmap with only 1 level #4255

adelavega opened this issue Feb 2, 2024 · 10 comments · Fixed by #4256
Labels
Bug for bug reports

Comments

@adelavega
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Operating system

  • Linux
  • Mac
  • Windows

Operating system version

For example one of the following:

  • Linux Ubuntu 22.04
  • Mac OS Version 12 "monterey"
  • Windows 11

Python version

  • 3.12
  • 3.11
  • 3.10
  • 3.9
  • 3.8

nilearn version

0.10.3

Expected behavior

Plot ROI as usual

Current behavior & error messages

Given an image with only one level (i.e. 0s and 1s), and a custom cmap, plot_roi throws an error constructing the color_bar.

<ipython-input-14-60f0ec2e3bf9> in <module>
----> 1 niplt.plot_roi(img, alpha=0.8, colorbar=True, cmap=cmap)

~/anaconda3/lib/python3.9/site-packages/nilearn/plotting/img_plotting.py in plot_roi(roi_img, bg_img, cut_coords, output_file, display_mode, figure, axes, title, annotate, draw_cross, black_bg, threshold, alpha, cmap, dim, colorbar, cbar_tick_format, vmin, vmax, resampling_interpolation, view_type, linewidths, radiological, **kwargs)
    921     )
    922 
--> 923     display = _plot_img_with_bg(
    924         img=roi_img,
    925         bg_img=bg_img,

~/anaconda3/lib/python3.9/site-packages/nilearn/plotting/img_plotting.py in _plot_img_with_bg(img, bg_img, cut_coords, output_file, display_mode, colorbar, figure, axes, title, threshold, annotate, draw_cross, black_bg, vmin, vmax, bg_vmin, bg_vmax, interpolation, display_factory, cbar_vmin, cbar_vmax, cbar_tick_format, brain_color, decimals, radiological, **kwargs)
    266 
    267     if img is not None and img is not False:
--> 268         display.add_overlay(
    269             new_img_like(img, data, affine),
    270             threshold=threshold,

~/anaconda3/lib/python3.9/site-packages/nilearn/plotting/displays/_slicers.py in add_overlay(self, img, threshold, colorbar, cbar_tick_format, cbar_vmin, cbar_vmax, **kwargs)
    314         # look at test_img_plotting.test_outlier_cut_coords.
    315         if colorbar and ims:
--> 316             self._show_colorbar(
    317                 ims[0].cmap, ims[0].norm, cbar_vmin, cbar_vmax, threshold
    318             )

~/anaconda3/lib/python3.9/site-packages/nilearn/plotting/displays/_slicers.py in _show_colorbar(self, cmap, norm, cbar_vmin, cbar_vmax, threshold)
    551                 "Custom cmap", cmaplist, our_cmap.N
    552             )
--> 553         self._cbar = ColorbarBase(
    554             self._colorbar_ax,
    555             ticks=ticks,

~/anaconda3/lib/python3.9/site-packages/matplotlib/colorbar.py in __init__(self, ax, mappable, cmap, norm, alpha, values, boundaries, orientation, ticklocation, extend, spacing, ticks, format, drawedges, extendfrac, extendrect, label, location)
    409         else:
    410             self._formatter = format  # Assume it is a Formatter or None
--> 411         self._draw_all()
    412 
    413         if isinstance(mappable, contour.ContourSet) and not mappable.filled:

~/anaconda3/lib/python3.9/site-packages/matplotlib/colorbar.py in _draw_all(self)
    535         self.vmin, self.vmax = self._boundaries[self._inside][[0, -1]]
    536         # Compute the X/Y mesh.
--> 537         X, Y = self._mesh()
    538         # draw the extend triangles, and shrink the inner axes to accommodate.
    539         # also adds the outline path to self.outline spine:

~/anaconda3/lib/python3.9/site-packages/matplotlib/colorbar.py in _mesh(self)
   1109         orientation.
   1110         """
-> 1111         y, _ = self._proportional_y()
   1112         # Use the vmin and vmax of the colorbar, which may not be the same
   1113         # as the norm. There are situations where the colormap has a

~/anaconda3/lib/python3.9/site-packages/matplotlib/colorbar.py in _proportional_y(self)
   1249         # make the lower and upper extend lengths proportional to the lengths
   1250         # of the first and last boundary spacing (if extendfrac='auto'):
-> 1251         automin = yscaled[1] - yscaled[0]
   1252         automax = yscaled[-1] - yscaled[-2]
   1253         extendlength = [0, 0]

IndexError: index 1 is out of bounds for axis 0 with size 1

Steps and code to reproduce bug

# Paste your code here

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

array_data = np.arange(24, dtype=np.int16).reshape((2, 3, 4))
array_data[:] = 0
array_data[0, 1, 1] = 1
affine = np.diag([1, 2, 3, 1])
img = nib.Nifti1Image(array_data, affine)
clust_ids = list(np.unique(img.get_fdata())[1:])
cmap = plt.cm.get_cmap("tab20", len(clust_ids))
cmap = plt.cm.get_cmap("tab20", len(clust_ids))
niplt.plot_roi(img, alpha=0.8, colorbar=True, cmap=cmap)
@adelavega adelavega added the Bug for bug reports label Feb 2, 2024
@adelavega
Copy link
Contributor Author

This worked in previous versions.

I suspect this was brought on by https://github.com/nilearn/nilearn/pull/2887 but I can't say for certain.

I stared at it for a while, and I couldn't quite spot the change that would bring on the error.

I will say the tick values looked suspect, as they included a negative value. But modifying that line did not fix the issue.

@adelavega
Copy link
Contributor Author

I haven't gotten that far, but it looks like the norm that is returned in the ims of _map_show differs, such that previously norm(1) returned 0, and now it returns 1.

@Remi-Gau
Copy link
Collaborator

Remi-Gau commented Feb 2, 2024

@adelavega
thanks for the bug report: will git bisect to confirm where the regression happened

@Remi-Gau
Copy link
Collaborator

Remi-Gau commented Feb 2, 2024

# Paste your code here

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


def main():
    array_data = np.arange(24, dtype=np.int16).reshape((2, 3, 4))
    array_data[:] = 0
    array_data[0, 1, 1] = 1
    affine = np.diag([1, 2, 3, 1])

    img = nib.Nifti1Image(array_data, affine)

    clust_ids = list(np.unique(img.get_fdata())[1:])

    cmap = plt.cm.get_cmap("tab20", len(clust_ids))
    cmap = plt.cm.get_cmap("tab20", len(clust_ids))

    try:
        niplt.plot_roi(img, alpha=0.8, colorbar=True, cmap=cmap)
        print("good")
        return 0
    except IndexError:
        print("bad")
        return 1
    except Exception:
        print("skipped")
        return 125

if __name__ == "__main__":
    exit(main())
git bisect start
git bisect good 0.10.2
git bisect bad 0.10.3
git bisect run python tmp.py

c018480 is the first bad commit

[ENH] Allow setting vmin in plot_glass_brain and plot_stat_map (#3993)

@Remi-Gau
Copy link
Collaborator

Remi-Gau commented Feb 2, 2024

OK adding back this thresholding fixed the bug

@Remi-Gau
Copy link
Collaborator

Remi-Gau commented Feb 2, 2024

@adelavega

once this is fixed, let me know if we should plan a bug fix release in case this is mission critical for some other downstream package.

@adelavega
Copy link
Contributor Author

Remi, amazing! I need to learn your debugging skills because I went down a rabbit hole last night :)

This does cause https://github.com/neurostuff/nimare tests to fail currently. It's not extremely urgent although I would imagine a cmap with a single level is not an uncommon usage pattern (e.g. for masks, clusters).

@Remi-Gau
Copy link
Collaborator

Remi-Gau commented Feb 2, 2024

Remi, amazing! I need to learn your debugging skills because I went down a rabbit hole last night :)

For the git bisect part:

https://www.youtube.com/watch?v=C2C7FTI8nB4

For the rest, print debugging did most of the work...

Actually I would not have minded a couple of extra unit tests for some internal methods, maybe worth adding in this PR.

@Remi-Gau
Copy link
Collaborator

Remi-Gau commented Feb 2, 2024

This does cause https://github.com/neurostuff/nimare tests to fail currently. It's not extremely urgent although I would imagine a cmap with a single level is not an uncommon usage pattern (e.g. for masks, clusters).

@bthirion
would it make sense to have a new release soonish to fix this?

@bthirion
Copy link
Member

bthirion commented Feb 2, 2024

I think so.

@Remi-Gau Remi-Gau added this to the Release 0.10.4 milestone Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug for bug reports
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants