From cfa74b4c1ab0e17b074c21604073d3152a4a0018 Mon Sep 17 00:00:00 2001 From: Ali Hamdan Date: Sat, 23 Dec 2023 14:10:45 +0100 Subject: [PATCH] Add a missing raise in kdeplot and slightly improve lmplot signature (#3602) I noticed these while working on the stubs at typeshed. They felt small enough to include in a single PR. 1. There is a missing `raise ` before `TypeError(msg)` in `kde_plot` 2. The `data` parameter of `lmplot` is required but it has a default of `None`. 20 lines later there is a check that raises a `TypeError` if `data is None`. Removing the default makes sense here to signal to the user that they must pass this argument. It also has two more advantages. The first is that your IDE will now helpfully yell at you if you don't pass the argument. The second is minor but it will allow us to keep running the stub tests for this function at typeshed where we removed the default which made the stub go out of sync with the runtime implementation so we had to skip its tests to pass CI. Note that there is no user facing change here as calling the function without passing `data` still raises a `TypeError` with a clear message. --- seaborn/distributions.py | 2 +- seaborn/regression.py | 2 +- tests/test_distributions.py | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/seaborn/distributions.py b/seaborn/distributions.py index 4953f01d59..e3d57c9a42 100644 --- a/seaborn/distributions.py +++ b/seaborn/distributions.py @@ -1593,7 +1593,7 @@ def kdeplot( # Handle (past) deprecation of `data2` if "data2" in kwargs: msg = "`data2` has been removed (replaced by `y`); please update your code." - TypeError(msg) + raise TypeError(msg) # Handle deprecation of `vertical` vertical = kwargs.pop("vertical", None) diff --git a/seaborn/regression.py b/seaborn/regression.py index 5a16fc96ac..5e5503a422 100644 --- a/seaborn/regression.py +++ b/seaborn/regression.py @@ -574,7 +574,7 @@ def lineplot(self, ax, kws): def lmplot( - data=None, *, + data, *, x=None, y=None, hue=None, col=None, row=None, palette=None, col_wrap=None, height=5, aspect=1, markers="o", sharex=None, sharey=None, hue_order=None, col_order=None, row_order=None, diff --git a/tests/test_distributions.py b/tests/test_distributions.py index e5f5c4aad8..0df1a15beb 100644 --- a/tests/test_distributions.py +++ b/tests/test_distributions.py @@ -920,6 +920,10 @@ def test_legend(self, long_df): assert ax.legend_ is None + def test_replaced_kws(self, long_df): + with pytest.raises(TypeError, match=r"`data2` has been removed"): + kdeplot(data=long_df, x="x", data2="y") + class TestKDEPlotBivariate: