Skip to content

Commit

Permalink
Fix: shifting margins caused by multiple rugplots (#2953)
Browse files Browse the repository at this point in the history
* Fix: shifting margins caused by multiple rugplots

This fix prevents margins from increasing when multiple rugplots are
added to the same `ax`, even if `expand_margins` is `False`.

As a minimum reproducible example:

```py
import seaborn as sns; sns.set()
import numpy as np
import matplotlib.pyplot as plt

values = np.linspace(start=0, stop=1, num=5)
ax = sns.lineplot(x=values, y=values)
sns.rugplot(x=values, ax=ax)

ylim = ax.get_ylim()

for _ in range(4):
    sns.rugplot(x=values, ax=ax, expand_margins=False)

if not all(a == b for a, b in zip(ylim, ax.get_ylim())):
    print(f'{ylim} != {ax.get_ylim()}')
plt.suptitle("Example showing that multiple rugplots cause issues")
plt.show()
```

Running the above code:

```sh
$ pip install seaborn numpy matplotlib; python3 test.py
(-0.1, 1.1) != (-0.61051, 1.14641)
```

This bug was caused by how seaborn detects the correct colors to use. In
`seaborn/utils.py`, in method `_default_color`, the following line used
to resolve to `ax.plot([], [], **kws)`:

```py
scout, = method([], [], **kws)
```
But matplotlib has the parameters `scalex` and `scaley` of `ax.plot` set
to `True` by default. Matplotlib would see that the rug was already on
the `ax` from the previous call to `sns.rugplot`, and so it would
rescale the x and y axes. This caused the content of the plot to take up
less and less space, with larger and larger margins as more rugplots
were added.

The fix sets `scalex` and `scaley` both to `False`, since this plot
method is a null-plot and is only used to check what colours should be
used:
```py
scout, = method([], [], scalex=False, scaley=False, **kws)
```

The above line is within an if-elif-else branch, but the other branches
do not suffer this bug and so no change is needed for them.

An additional unit test was also added to catch this bug:
`test_multiple_rugs`.

* Update PR based on comments

* Remove unused import
  • Loading branch information
beyarkay authored Aug 13, 2022
1 parent 2c3ea6f commit 7081223
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 1 deletion.
2 changes: 2 additions & 0 deletions doc/whatsnew/v0.12.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ Other updates

- |Fix| Improved robustness to numerical errors in :func:`kdeplot` (:pr:`2862`).

- |Fix| Fixed a bug where :func:`rugplot` was ignoring expand_margins=False (:pr:`2953`).

- |Defaults| The `patch.facecolor` rc param is no longer set by :func:`set_palette` (or :func:`set_theme`). This should have no general effect, because the matplotlib default is now `"C0"` (:pr:`2906`).

- |Deps| Made `scipy` an optional dependency and added `pip install seaborn[stats]` as a method for ensuring the availability of compatible `scipy` and `statsmodels` libraries at install time. This has a few minor implications for existing code, which are explained in the Github pull request (:pr:`2398`).
Expand Down
2 changes: 1 addition & 1 deletion seaborn/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def _default_color(method, hue, color, kws):

elif method.__name__ == "plot":

scout, = method([], [], **kws)
scout, = method([], [], scalex=False, scaley=False, **kws)
color = scout.get_color()
scout.remove()

Expand Down
10 changes: 10 additions & 0 deletions tests/test_distributions.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,16 @@ def test_expand_margins(self, flat_array):
assert x1 == x2
assert y1 + height * 2 == pytest.approx(y2)

def test_multiple_rugs(self):

values = np.linspace(start=0, stop=1, num=5)
ax = rugplot(x=values)
ylim = ax.get_ylim()

rugplot(x=values, ax=ax, expand_margins=False)

assert ylim == ax.get_ylim()

def test_matplotlib_kwargs(self, flat_series):

lw = 2
Expand Down

0 comments on commit 7081223

Please sign in to comment.