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

[FIX] Allow both vmin and threshold argument values to be used in plotting functions using colorscale #3945

Merged
merged 4 commits into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/changes/latest.rst
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ Fixes

- Fix ``fit_transform`` behavior to match when ``fit`` method is passed image data (:gh:`3897` by `Yasmin Mzayek`_)

- Allow using both vmin and threshold with "plotly" engine to be consistent with "matplotlib" behavior (:gh:`3945` by `Yasmin Mzayek`_)

Enhancements
------------

Expand Down
5 changes: 0 additions & 5 deletions nilearn/plotting/js_plotting_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,6 @@ def colorscale(cmap, values, threshold=None, symmetric_cmap=True,
if symmetric_cmap and vmin is not None:
warnings.warn('vmin cannot be chosen when cmap is symmetric')
vmin = None
if threshold is not None:
if vmin is not None:
warnings.warn('choosing both vmin and a threshold is not allowed; '
'setting vmin to 0')
vmin = 0
if vmax is None:
vmax = abs_values.max()
# cast to float to avoid TypeError if vmax is a numpy boolean
Expand Down
15 changes: 6 additions & 9 deletions nilearn/plotting/tests/test_js_plotting_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,13 @@ def test_colorscale_symmetric_cmap(vmin, vmax):


@pytest.fixture
def expected_vmin_vmax(values, vmax, vmin, threshold):
def expected_vmin_vmax(values, vmax, vmin):
"""Returns expected vmin and vmax."""
if threshold is None:
if vmax is None:
return (min(values), max(values))
if min(values) < 0:
return (-vmax, vmax)
return (min(values), vmax) if vmin is None else (vmin, vmax)
else:
return (0, vmax)
if vmax is None:
return (min(values), max(values))
if min(values) < 0:
return (-vmax, vmax)
return (min(values), vmax) if vmin is None else (vmin, vmax)


@pytest.mark.parametrize(
Expand Down
22 changes: 22 additions & 0 deletions nilearn/plotting/tests/test_surf_plotting.py
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,28 @@ def test_plot_surf_roi_error(engine):
plot_surf_roi(mesh, roi_map=roi_idx, engine=engine)


@pytest.mark.parametrize(
"kwargs", [{"vmin": 2}, {"vmin": 2, "threshold": 5}, {"threshold": 5}]
)
def test_plot_surf_roi_colorbar_vmin_equal_across_engines(kwargs):
"""See issue https://github.com/nilearn/nilearn/issues/3944"""
if not PLOTLY_INSTALLED:
pytest.skip("Plotly is not installed; required for this test.")
mesh = generate_surf()
roi_map = np.arange(0, len(mesh[0]))

mpl_plot = plot_surf_roi(
mesh, roi_map=roi_map, colorbar=True, engine="matplotlib", **kwargs
)
plotly_plot = plot_surf_roi(
mesh, roi_map=roi_map, colorbar=True, engine="plotly", **kwargs
)
assert (
int(mpl_plot.axes[-1].get_yticklabels()[0].get_text())
== plotly_plot.figure.data[1]["cmin"]
)


def test_plot_img_on_surf_hemispheres_and_orientations(img_3d_mni):
nii = img_3d_mni
# Check that all combinations of 1D or 2D hemis and orientations work.
Expand Down