From b3e35ac510b232cf1936496a2238921bcdb6663a Mon Sep 17 00:00:00 2001 From: Marc Van den Bossche Date: Thu, 3 Nov 2022 12:14:27 +0100 Subject: [PATCH 1/8] Captures 0 and 1 --- lib/matplotlib/gridspec.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/gridspec.py b/lib/matplotlib/gridspec.py index 06dd3f19f68a..32ee7c306ef9 100644 --- a/lib/matplotlib/gridspec.py +++ b/lib/matplotlib/gridspec.py @@ -276,9 +276,9 @@ def subplots(self, *, sharex=False, sharey=False, squeeze=True, raise ValueError("GridSpec.subplots() only works for GridSpecs " "created with a parent figure") - if isinstance(sharex, bool): + if isinstance(sharex, bool) or sharex == 1 or sharex == 0: sharex = "all" if sharex else "none" - if isinstance(sharey, bool): + if isinstance(sharey, bool) or sharey == 1 or sharey == 0: sharey = "all" if sharey else "none" # This check was added because it is very easy to type # `subplots(1, 2, 1)` when `subplot(1, 2, 1)` was intended. From 2a2732ae219cacfddb834b6708c0f03559ef8052 Mon Sep 17 00:00:00 2001 From: Marc Van den Bossche Date: Thu, 3 Nov 2022 16:57:15 +0100 Subject: [PATCH 2/8] Imporoved error message --- lib/matplotlib/gridspec.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/matplotlib/gridspec.py b/lib/matplotlib/gridspec.py index 32ee7c306ef9..9869c0f24124 100644 --- a/lib/matplotlib/gridspec.py +++ b/lib/matplotlib/gridspec.py @@ -290,7 +290,7 @@ def subplots(self, *, sharex=False, sharey=False, squeeze=True, _api.warn_external( "sharex argument to subplots() was an integer. Did you " "intend to use subplot() (without 's')?") - _api.check_in_list(["all", "row", "col", "none"], + _api.check_in_list(["all", "row", "col", "none", 0, 1, False, True], sharex=sharex, sharey=sharey) if subplot_kw is None: subplot_kw = {} From 44d1f236658b097ed1e782bc508ea9db66355a9a Mon Sep 17 00:00:00 2001 From: Marc Van den Bossche Date: Fri, 4 Nov 2022 11:21:42 +0100 Subject: [PATCH 3/8] Added 0 and 1 to subplot test --- lib/matplotlib/tests/test_subplots.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/matplotlib/tests/test_subplots.py b/lib/matplotlib/tests/test_subplots.py index 732418f19e2f..462dc55d8a8d 100644 --- a/lib/matplotlib/tests/test_subplots.py +++ b/lib/matplotlib/tests/test_subplots.py @@ -84,7 +84,7 @@ def test_shared(): plt.close(f) # test all option combinations - ops = [False, True, 'all', 'none', 'row', 'col'] + ops = [False, True, 'all', 'none', 'row', 'col', 0, 1] for xo in ops: for yo in ops: f, ((a1, a2), (a3, a4)) = plt.subplots(2, 2, sharex=xo, sharey=yo) From cc89599fef156b00160f8588f1f2bdcef37965c1 Mon Sep 17 00:00:00 2001 From: Marc Van den Bossche Date: Fri, 4 Nov 2022 11:24:22 +0100 Subject: [PATCH 4/8] Added exception test --- lib/matplotlib/tests/test_subplots.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/matplotlib/tests/test_subplots.py b/lib/matplotlib/tests/test_subplots.py index 462dc55d8a8d..1584455daa4b 100644 --- a/lib/matplotlib/tests/test_subplots.py +++ b/lib/matplotlib/tests/test_subplots.py @@ -148,6 +148,8 @@ def test_exceptions(): plt.subplots(2, 2, sharex='blah') with pytest.raises(ValueError): plt.subplots(2, 2, sharey='blah') + with pytest.raises(ValueError): + plt.subplots(2, 2, sharey=2) @image_comparison(['subplots_offset_text']) From 805104451516eb4ba1d1c5b3b0e8b708f02ee83e Mon Sep 17 00:00:00 2001 From: Marc Van den Bossche Date: Tue, 8 Nov 2022 17:35:18 +0100 Subject: [PATCH 5/8] Removed isinstance(share*, Integral) + added suggested changes --- lib/matplotlib/gridspec.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/lib/matplotlib/gridspec.py b/lib/matplotlib/gridspec.py index 9869c0f24124..ab934fbc459e 100644 --- a/lib/matplotlib/gridspec.py +++ b/lib/matplotlib/gridspec.py @@ -276,20 +276,15 @@ def subplots(self, *, sharex=False, sharey=False, squeeze=True, raise ValueError("GridSpec.subplots() only works for GridSpecs " "created with a parent figure") - if isinstance(sharex, bool) or sharex == 1 or sharex == 0: + if isinstance(sharex, str): + pass + else: sharex = "all" if sharex else "none" - if isinstance(sharey, bool) or sharey == 1 or sharey == 0: + if isinstance(sharey, str): + pass + else: sharey = "all" if sharey else "none" - # This check was added because it is very easy to type - # `subplots(1, 2, 1)` when `subplot(1, 2, 1)` was intended. - # In most cases, no error will ever occur, but mysterious behavior - # will result because what was intended to be the subplot index is - # instead treated as a bool for sharex. This check should go away - # once sharex becomes kwonly. - if isinstance(sharex, Integral): - _api.warn_external( - "sharex argument to subplots() was an integer. Did you " - "intend to use subplot() (without 's')?") + _api.check_in_list(["all", "row", "col", "none", 0, 1, False, True], sharex=sharex, sharey=sharey) if subplot_kw is None: From 1015ffd47a2c6c1d11751e2a8e6725b5c396f847 Mon Sep 17 00:00:00 2001 From: Marc Van den Bossche Date: Sun, 20 Nov 2022 20:57:39 +0100 Subject: [PATCH 6/8] Better ifs and less tests --- lib/matplotlib/gridspec.py | 10 +++------- lib/matplotlib/tests/test_subplots.py | 2 -- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/lib/matplotlib/gridspec.py b/lib/matplotlib/gridspec.py index ab934fbc459e..59610a7b4d4c 100644 --- a/lib/matplotlib/gridspec.py +++ b/lib/matplotlib/gridspec.py @@ -276,16 +276,12 @@ def subplots(self, *, sharex=False, sharey=False, squeeze=True, raise ValueError("GridSpec.subplots() only works for GridSpecs " "created with a parent figure") - if isinstance(sharex, str): - pass - else: + if not isinstance(sharex, str): sharex = "all" if sharex else "none" - if isinstance(sharey, str): - pass - else: + if not isinstance(sharey, str): sharey = "all" if sharey else "none" - _api.check_in_list(["all", "row", "col", "none", 0, 1, False, True], + _api.check_in_list(["all", "row", "col", "none", False, True], sharex=sharex, sharey=sharey) if subplot_kw is None: subplot_kw = {} diff --git a/lib/matplotlib/tests/test_subplots.py b/lib/matplotlib/tests/test_subplots.py index 1584455daa4b..462dc55d8a8d 100644 --- a/lib/matplotlib/tests/test_subplots.py +++ b/lib/matplotlib/tests/test_subplots.py @@ -148,8 +148,6 @@ def test_exceptions(): plt.subplots(2, 2, sharex='blah') with pytest.raises(ValueError): plt.subplots(2, 2, sharey='blah') - with pytest.raises(ValueError): - plt.subplots(2, 2, sharey=2) @image_comparison(['subplots_offset_text']) From 53e7dc8796137780ba2fb743553b85c67e6fda22 Mon Sep 17 00:00:00 2001 From: Marc Van den Bossche Date: Sun, 20 Nov 2022 21:00:39 +0100 Subject: [PATCH 7/8] Change in behaviour : removed tests --- lib/matplotlib/tests/test_subplots.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/matplotlib/tests/test_subplots.py b/lib/matplotlib/tests/test_subplots.py index 462dc55d8a8d..aad646d63cbf 100644 --- a/lib/matplotlib/tests/test_subplots.py +++ b/lib/matplotlib/tests/test_subplots.py @@ -142,14 +142,6 @@ def test_shared_and_moved(): check_visible([a1], [False], [True]) -def test_exceptions(): - # TODO should this test more options? - with pytest.raises(ValueError): - plt.subplots(2, 2, sharex='blah') - with pytest.raises(ValueError): - plt.subplots(2, 2, sharey='blah') - - @image_comparison(['subplots_offset_text']) def test_subplots_offsettext(): x = np.arange(0, 1e10, 1e9) From ca12ec475e4e4981e131a9e26b10d9a795b9723c Mon Sep 17 00:00:00 2001 From: Marc Van den Bossche Date: Thu, 15 Dec 2022 18:53:04 +0100 Subject: [PATCH 8/8] Restored exception test --- lib/matplotlib/tests/test_subplots.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/matplotlib/tests/test_subplots.py b/lib/matplotlib/tests/test_subplots.py index aad646d63cbf..462dc55d8a8d 100644 --- a/lib/matplotlib/tests/test_subplots.py +++ b/lib/matplotlib/tests/test_subplots.py @@ -142,6 +142,14 @@ def test_shared_and_moved(): check_visible([a1], [False], [True]) +def test_exceptions(): + # TODO should this test more options? + with pytest.raises(ValueError): + plt.subplots(2, 2, sharex='blah') + with pytest.raises(ValueError): + plt.subplots(2, 2, sharey='blah') + + @image_comparison(['subplots_offset_text']) def test_subplots_offsettext(): x = np.arange(0, 1e10, 1e9)