seaborn icon indicating copy to clipboard operation
seaborn copied to clipboard

FacetGrid: despine can contradict with style settings

Open asongtoruin opened this issue 3 years ago • 5 comments

Hello! Bit of an edge case here, but one that might be worth thinking about. When setting despine=True for FacetGrid, this can lead to slightly confusing results if the style currently in use already sets the bottom and / or left spine to be removed.

The below example just uses rcParams as a proxy for some larger style:

import matplotlib.pyplot as plt
import seaborn as sns

plt.rcParams['axes.spines.left'] = False
plt.rcParams['axes.spines.bottom'] = False

tips = sns.load_dataset("tips")

Then, if we create a FacetGrid with the default despine=True, we end up with two spines drawn:

g = sns.FacetGrid(tips, col="time",  row="sex", despine=True)
g.map(sns.scatterplot, "total_bill", "tip")

Grid with left and bottom spines drawn on each axis

Whereas if we use despine=False, the left and bottom sides are not drawn:

g = sns.FacetGrid(tips, col="time",  row="sex", despine=False)
g.map(sns.scatterplot, "total_bill", "tip")

Grid with no spines on each axis

Which feels slightly contradictory to me. This seems to be happening because FacetGrid.despine() gets called with default parameters when despine=True, which in turn calls utils.despine with the corresponding figure and its default parameters where left=False, bottom=False.

Possible solutions for this might include:

  1. Rephrasing the documentation (or, less likely, renaming the parameter) to make it clearer that setting despine=True actually makes it so that the left and bottom spines only are shown
  2. Changing the behaviour associated with despine=True so that the left and bottom spines are inferred from (the inverse of) plt.rcParams values instead
  3. Nothing, because it is a bit of an edge case.

Happy to work up a pull request if you think 1) or 2) would be useful. Thanks!

asongtoruin avatar Aug 23 '21 17:08 asongtoruin

Hm, yes, good question. When despine and FacetGrid were written, spine visibility was not something you could configure through matplotlib rcparams so this wasn't so much of an issue.

Note that I can't replicate your second example, and I'm not sure why you think it's inconsistent:

with plt.rc_context({"axes.spines.left": False, "axes.spines.bottom": False}):
    g = sns.FacetGrid(tips, col="time",  row="sex", despine=False)
    g.map(sns.scatterplot, "total_bill", "tip")

image

I'm not sure why you're getting a plot with no spines, but this behavior seems correct to me; setting despine=False in FacetGrid simply does not modify spine visibility. Note that the result of setting the rcparams that way is a bit different from despine(left=True, bottom=True, top=False, right=False), which would also move the ticks to the top and right.

But changing sns.despine to work better with the rcparams seems reasonable.

I suppose there are two options:

  • Change the interpretation of {side}=False to mean "do not modify visibility" rather than meaning the inverse visibility state. Technically a breaking change, though I am not sure whether it would actually affect anyone in practice.
  • Change the default for left and bottom to None, and have None mean "do not modify visibility" with True and False having the existing semantics.

In both cases, one would need to think about the existing logic/implementation of the rule for moving the ticks.

I would probably lean toward the former options, because it's weird for a function named despine to (apparently) add spines back.

mwaskom avatar Aug 23 '21 21:08 mwaskom

Sorry - I was trying to simplify the example and forgot to re-run the second cell before taking the screenshot. You're correct in that it should have the top and right spines.


With regards changing {side}=False to mean "do not modify visibility", would this not affect anyone who doesn't change from matplotlib's default style? I think that might affect a lot of people, as people might expect the "seaborn style" with the open sides. But I agree with you - despine as a function adding spines back in would be confusing.

If you're comfortable accepting a breaking change, a parameter name change might be cleaner, e.g.

FacetGrid(..., autospine=True)

This would perhaps make it more obvious that setting it to False keeps whatever the user currently has set, while also not introducing any additional ambiguity into the despine method. Would maybe need to add in some deprecation warnings and keep the despine and (e.g.) autospine parameters as synonyms to begin with.

asongtoruin avatar Aug 24 '21 13:08 asongtoruin

With regards changing {side}=False to mean "do not modify visibility", would this not affect anyone who doesn't change from matplotlib's default style?

I don't think so. The parameter defaults would remain the same, so top and right would get removed, and bottom and left would maintain the visibility setting in the rcparams, which is visible.

mwaskom avatar Aug 24 '21 23:08 mwaskom

Ah yes, you're right.

I think one way to implement that could be to change

https://github.com/mwaskom/seaborn/blob/091f4c0e4f3580a8060de5596fa64c1ff9454dc5/seaborn/utils.py#L326

to be

is_visible = (not locals()[side]) and plt.rcParams[f"axes.spines.{side}"]

Which I think would evaluate like this:

visible? rcParams {side}=True rcParams {side}=False
despine {side}=True False False
despine {side}=False True False

In other words, the only way a side would be set to be visible would be if that side wasn't being despined and the style settings currently had it set to be visible. Which I think checks out with what you were suggesting?

The ticks do add a slight complication, but I think the solution may be the same. For example, this line:

https://github.com/mwaskom/seaborn/blob/091f4c0e4f3580a8060de5596fa64c1ff9454dc5/seaborn/utils.py#L336

will be checking to see if left is being removed and right is not removed. The extra bit to check now, I think, will be whether the right side is visible, so that may be as simple as:

if left and (not right) and plt.rcParams["axes.spines.right"]:

with the same logic applying for the top and bottom tick comparison.

Any thoughts? I can work this into a PR later if you think it's the right way to go.

asongtoruin avatar Aug 25 '21 08:08 asongtoruin

I kinda feel like we should decouple "removal" and "visibility" and rewrite the code that way, so that the ticks only get moved to the opposite side if their spine is getting actively removed by that call to despine, rather than their spines being invisible at that point. In practice I'm not sure it would ever really matter much (I guess you'd need your rcparams to have the left spines off, the right spines on, and call despine with right=False?) but seems conceptually clearer.

The code quality in despine is overall not that great, so I guess it could get spruced up here, but if you're more comfortable making a targeted change to implement the basic change to what {side}=False means, that's totally fine.

Please be sure to add (ideally, start with) a test.

mwaskom avatar Aug 28 '21 14:08 mwaskom