From 35226f15fb6bffb8b8572c9ab849428c9b28c92f Mon Sep 17 00:00:00 2001 From: Shaokun Xie Date: Fri, 6 Sep 2024 16:31:26 +0800 Subject: [PATCH 01/12] BUG: fix orientation of colorbar in multiplanel plot --- yt/visualization/base_plot_types.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/yt/visualization/base_plot_types.py b/yt/visualization/base_plot_types.py index 377e316be91..25221752b7d 100644 --- a/yt/visualization/base_plot_types.py +++ b/yt/visualization/base_plot_types.py @@ -341,7 +341,11 @@ def _set_axes(self) -> None: self.image.axes.set_facecolor(self.colorbar_handler.background_color) self.cax.tick_params(which="both", direction="in") - self.cb = self.figure.colorbar(self.image, self.cax) + + # For creating a multipanel plot by ImageGrid, we need the location keyword + self.cb = self.figure.colorbar( + self.image, self.cax, location=getattr(self.cax, "orientation", None) + ) cb_axis: Axis if self.cb.orientation == "vertical": From c47732d09918a46bcf9e7dfa2a8e315c660bafd8 Mon Sep 17 00:00:00 2001 From: Shaokun Xie Date: Sat, 7 Sep 2024 21:04:39 +0800 Subject: [PATCH 02/12] add check for matplotlib version --- yt/visualization/base_plot_types.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/yt/visualization/base_plot_types.py b/yt/visualization/base_plot_types.py index 25221752b7d..948bef7ed7d 100644 --- a/yt/visualization/base_plot_types.py +++ b/yt/visualization/base_plot_types.py @@ -342,10 +342,19 @@ def _set_axes(self) -> None: self.cax.tick_params(which="both", direction="in") - # For creating a multipanel plot by ImageGrid, we need the location keyword - self.cb = self.figure.colorbar( - self.image, self.cax, location=getattr(self.cax, "orientation", None) - ) + # For creating a multipanel plot by ImageGrid + # we may need the location keyword, which was introduced since Matplotlib 3.7.0 + cb_location = getattr(self.cax, "orientation", None) + if matplotlib.__version_info__ >= (3, 7): + cb_kwargs = {"location": cb_location} + else: + cb_kwargs = {} + if cb_location in ["top", "bottom"]: + warnings.warn( + "Colorbar orientation would be wrong in the current Matplotlib version (< 3.7.0)", + stacklevel=2, + ) + self.cb = self.figure.colorbar(self.image, self.cax, **cb_kwargs) cb_axis: Axis if self.cb.orientation == "vertical": From cf4e14ddc272848d5f517a6b83a9f8f7fab1e23a Mon Sep 17 00:00:00 2001 From: Shaokun Xie Date: Sun, 8 Sep 2024 00:02:19 +0800 Subject: [PATCH 03/12] TST: add test for colorbar orientation in multi-panel plot --- .../tests/test_image_comp_2D_plots.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/yt/visualization/tests/test_image_comp_2D_plots.py b/yt/visualization/tests/test_image_comp_2D_plots.py index 254c8e5338a..8fab102e916 100644 --- a/yt/visualization/tests/test_image_comp_2D_plots.py +++ b/yt/visualization/tests/test_image_comp_2D_plots.py @@ -147,6 +147,24 @@ def test_particleprojectionplot_set_colorbar_properties(): return p.plots[field].figure +@pytest.mark.skipif( + mpl.__version_info__ < (3, 7), + reason="colorbar cannot currently be set horizontal in multi-panel plot with matplotlib older than 3.7.0", +) +@pytest.mark.parametrize("cbar_location", ["top", "bottom", "left", "right"]) +@pytest.mark.mpl_image_compare +def test_multipanelplot_colorbar_orientation(cbar_location): + ds = fake_random_ds(16) + fields = [ + ("gas", "density"), + ("gas", "velocity_x"), + ("gas", "velocity_y"), + ("gas", "velocity_magnitude"), + ] + p = SlicePlot(ds, "z", fields) + return p.export_to_mpl_figure((2, 2), cbar_location=cbar_location) + + class TestProfilePlot: @classmethod def setup_class(cls): From d38bca2a3b9b9b27be04b6be0e1359ed3f09fc44 Mon Sep 17 00:00:00 2001 From: Shaokun Xie Date: Mon, 9 Sep 2024 17:47:52 +0800 Subject: [PATCH 04/12] add test for warning and refactor tests for multipanel plot --- .../tests/test_image_comp_2D_plots.py | 50 +++++++++++++------ 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/yt/visualization/tests/test_image_comp_2D_plots.py b/yt/visualization/tests/test_image_comp_2D_plots.py index 8fab102e916..41857fb5ac1 100644 --- a/yt/visualization/tests/test_image_comp_2D_plots.py +++ b/yt/visualization/tests/test_image_comp_2D_plots.py @@ -147,22 +147,40 @@ def test_particleprojectionplot_set_colorbar_properties(): return p.plots[field].figure -@pytest.mark.skipif( - mpl.__version_info__ < (3, 7), - reason="colorbar cannot currently be set horizontal in multi-panel plot with matplotlib older than 3.7.0", -) -@pytest.mark.parametrize("cbar_location", ["top", "bottom", "left", "right"]) -@pytest.mark.mpl_image_compare -def test_multipanelplot_colorbar_orientation(cbar_location): - ds = fake_random_ds(16) - fields = [ - ("gas", "density"), - ("gas", "velocity_x"), - ("gas", "velocity_y"), - ("gas", "velocity_magnitude"), - ] - p = SlicePlot(ds, "z", fields) - return p.export_to_mpl_figure((2, 2), cbar_location=cbar_location) +class TestMultipanelPlot: + @classmethod + def setup_class(cls): + cls.fields = [ + ("gas", "density"), + ("gas", "velocity_x"), + ("gas", "velocity_y"), + ("gas", "velocity_magnitude"), + ] + cls.ds = fake_random_ds(16) + + @pytest.mark.skipif( + mpl.__version_info__ < (3, 7), + reason="colorbar cannot currently be set horizontal in multi-panel plot with matplotlib older than 3.7.0", + ) + @pytest.mark.parametrize("cbar_location", ["top", "bottom", "left", "right"]) + @pytest.mark.mpl_image_compare + def test_multipanelplot_colorbar_orientation_simple(self, cbar_location): + p = SlicePlot(self.ds, "z", self.fields) + return p.export_to_mpl_figure((2, 2), cbar_location=cbar_location) + + @pytest.mark.parametrize("cbar_location", ["top", "bottom"]) + def test_multipanelplot_colorbar_orientation_warning(self, cbar_location): + p = SlicePlot(self.ds, "z", self.fields) + if mpl.__version_info__ < (3, 7): + with pytest.warns( + UserWarning, + match=( + r"Colorbar orientation would be wrong in the current Matplotlib version \(\< 3\.7\.0\)" + ), + ): + p.export_to_mpl_figure((2, 2), cbar_location=cbar_location) + else: + p.export_to_mpl_figure((2, 2), cbar_location=cbar_location) class TestProfilePlot: From 29ae7a5d458131fc1b5071f9d3c80866c828fa6e Mon Sep 17 00:00:00 2001 From: Shaokun Xie Date: Mon, 9 Sep 2024 17:54:34 +0800 Subject: [PATCH 05/12] change stacklevel --- yt/visualization/base_plot_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yt/visualization/base_plot_types.py b/yt/visualization/base_plot_types.py index 948bef7ed7d..71d4a063950 100644 --- a/yt/visualization/base_plot_types.py +++ b/yt/visualization/base_plot_types.py @@ -352,7 +352,7 @@ def _set_axes(self) -> None: if cb_location in ["top", "bottom"]: warnings.warn( "Colorbar orientation would be wrong in the current Matplotlib version (< 3.7.0)", - stacklevel=2, + stacklevel=6, ) self.cb = self.figure.colorbar(self.image, self.cax, **cb_kwargs) From 9da531a1f7d15c8681186f2ed66b507d8512ada9 Mon Sep 17 00:00:00 2001 From: Shaokun Xie Date: Mon, 9 Sep 2024 18:33:05 +0800 Subject: [PATCH 06/12] fix type-checking --- yt/visualization/base_plot_types.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/yt/visualization/base_plot_types.py b/yt/visualization/base_plot_types.py index 71d4a063950..7af2b1857c2 100644 --- a/yt/visualization/base_plot_types.py +++ b/yt/visualization/base_plot_types.py @@ -346,15 +346,14 @@ def _set_axes(self) -> None: # we may need the location keyword, which was introduced since Matplotlib 3.7.0 cb_location = getattr(self.cax, "orientation", None) if matplotlib.__version_info__ >= (3, 7): - cb_kwargs = {"location": cb_location} + self.cb = self.figure.colorbar(self.image, self.cax, location=cb_location) else: - cb_kwargs = {} if cb_location in ["top", "bottom"]: warnings.warn( "Colorbar orientation would be wrong in the current Matplotlib version (< 3.7.0)", stacklevel=6, ) - self.cb = self.figure.colorbar(self.image, self.cax, **cb_kwargs) + self.cb = self.figure.colorbar(self.image, self.cax) cb_axis: Axis if self.cb.orientation == "vertical": From 42fd4eb437b8fbd33902939be5a4d2a6c948fe2c Mon Sep 17 00:00:00 2001 From: Shaokun Xie Date: Mon, 9 Sep 2024 20:13:59 +0800 Subject: [PATCH 07/12] fix wrong indentation --- yt/visualization/base_plot_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yt/visualization/base_plot_types.py b/yt/visualization/base_plot_types.py index 7af2b1857c2..1a458a7f831 100644 --- a/yt/visualization/base_plot_types.py +++ b/yt/visualization/base_plot_types.py @@ -353,7 +353,7 @@ def _set_axes(self) -> None: "Colorbar orientation would be wrong in the current Matplotlib version (< 3.7.0)", stacklevel=6, ) - self.cb = self.figure.colorbar(self.image, self.cax) + self.cb = self.figure.colorbar(self.image, self.cax) cb_axis: Axis if self.cb.orientation == "vertical": From 45cf7393ce0cfdea37661430b704e1ce586d0fc4 Mon Sep 17 00:00:00 2001 From: Shaokun Xie Date: Tue, 10 Sep 2024 21:18:24 +0800 Subject: [PATCH 08/12] BUG: fix horizontal colorbar labels are not properly formatted --- yt/visualization/base_plot_types.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/yt/visualization/base_plot_types.py b/yt/visualization/base_plot_types.py index 1a458a7f831..cc97ec8c229 100644 --- a/yt/visualization/base_plot_types.py +++ b/yt/visualization/base_plot_types.py @@ -540,7 +540,9 @@ def _get_labels(self): labels = super()._get_labels() cbax = self.cb.ax labels += cbax.yaxis.get_ticklabels() + labels += cbax.xaxis.get_ticklabels() labels += [cbax.yaxis.label, cbax.yaxis.get_offset_text()] + labels += [cbax.xaxis.label, cbax.xaxis.get_offset_text()] return labels def hide_axes(self, *, draw_frame=None): From b8158a8c8ece5bffb31a88b33a2c4275f9d4ebdb Mon Sep 17 00:00:00 2001 From: Shaokun Xie Date: Wed, 11 Sep 2024 01:00:27 +0800 Subject: [PATCH 09/12] suggested changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Clément Robert --- yt/visualization/base_plot_types.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/yt/visualization/base_plot_types.py b/yt/visualization/base_plot_types.py index cc97ec8c229..a85b65c30c6 100644 --- a/yt/visualization/base_plot_types.py +++ b/yt/visualization/base_plot_types.py @@ -538,11 +538,12 @@ def _toggle_colorbar(self, choice: bool): def _get_labels(self): labels = super()._get_labels() - cbax = self.cb.ax - labels += cbax.yaxis.get_ticklabels() - labels += cbax.xaxis.get_ticklabels() - labels += [cbax.yaxis.label, cbax.yaxis.get_offset_text()] - labels += [cbax.xaxis.label, cbax.xaxis.get_offset_text()] + if getattr(self.cb, "orientation", "vertical") == "horizontal": + cbaxis = self.cb.ax.xaxis + else: + cbaxis = self.cb.ax.yaxis + labels += cbaxis.get_ticklabels() + labels += [cbaxis.label, cbaxis.get_offset_text()] return labels def hide_axes(self, *, draw_frame=None): From e4a1096edf4f7c29569134208abbaaf65bc3c9df Mon Sep 17 00:00:00 2001 From: Shaokun Xie Date: Thu, 12 Sep 2024 19:59:57 +0800 Subject: [PATCH 10/12] upgrade images test baseline --- tests/pytest_mpl_baseline | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/pytest_mpl_baseline b/tests/pytest_mpl_baseline index 9162b1a3f1b..8a30c989a30 160000 --- a/tests/pytest_mpl_baseline +++ b/tests/pytest_mpl_baseline @@ -1 +1 @@ -Subproject commit 9162b1a3f1bdccbe398221fec8ace489e53078d9 +Subproject commit 8a30c989a30b00e681db645cc767cb9047508169 From 90bd62d0af9538c0a3be8f5747b6ad38ed7b946a Mon Sep 17 00:00:00 2001 From: Shaokun Xie Date: Fri, 13 Sep 2024 15:22:26 +0800 Subject: [PATCH 11/12] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Clément Robert --- yt/visualization/base_plot_types.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/yt/visualization/base_plot_types.py b/yt/visualization/base_plot_types.py index a85b65c30c6..94792c32d33 100644 --- a/yt/visualization/base_plot_types.py +++ b/yt/visualization/base_plot_types.py @@ -343,14 +343,15 @@ def _set_axes(self) -> None: self.cax.tick_params(which="both", direction="in") # For creating a multipanel plot by ImageGrid - # we may need the location keyword, which was introduced since Matplotlib 3.7.0 + # we may need the location keyword, which requires Matplotlib >= 3.7.0 cb_location = getattr(self.cax, "orientation", None) if matplotlib.__version_info__ >= (3, 7): self.cb = self.figure.colorbar(self.image, self.cax, location=cb_location) else: if cb_location in ["top", "bottom"]: warnings.warn( - "Colorbar orientation would be wrong in the current Matplotlib version (< 3.7.0)", + "Cannot properly set the orientation of colorbar. " + "Consider upgrading matplotlib to version 3.7 or newer", stacklevel=6, ) self.cb = self.figure.colorbar(self.image, self.cax) From 91fc3836766a8c251fc4f71a521bcdd970fc5e91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Robert?= Date: Fri, 13 Sep 2024 09:24:54 +0200 Subject: [PATCH 12/12] Update yt/visualization/tests/test_image_comp_2D_plots.py --- yt/visualization/tests/test_image_comp_2D_plots.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/yt/visualization/tests/test_image_comp_2D_plots.py b/yt/visualization/tests/test_image_comp_2D_plots.py index 41857fb5ac1..371256c0eb4 100644 --- a/yt/visualization/tests/test_image_comp_2D_plots.py +++ b/yt/visualization/tests/test_image_comp_2D_plots.py @@ -174,9 +174,7 @@ def test_multipanelplot_colorbar_orientation_warning(self, cbar_location): if mpl.__version_info__ < (3, 7): with pytest.warns( UserWarning, - match=( - r"Colorbar orientation would be wrong in the current Matplotlib version \(\< 3\.7\.0\)" - ), + match="Cannot properly set the orientation of colorbar.", ): p.export_to_mpl_figure((2, 2), cbar_location=cbar_location) else: