Skip to content

Commit

Permalink
Add a missing raise in kdeplot and slightly improve lmplot signature (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
hamdanal authored Dec 23, 2023
1 parent 5051ede commit cfa74b4
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 2 deletions.
2 changes: 1 addition & 1 deletion seaborn/distributions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion seaborn/regression.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions tests/test_distributions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down

0 comments on commit cfa74b4

Please sign in to comment.