Skip to content

Commit

Permalink
Fix inferred y orientation with computed x variable (#2818)
Browse files Browse the repository at this point in the history
* Fix inferred y orientation with computed x variable

I'm not totally satisfied with this, but mutating the layer dict
(the other option) doesn't seem much cleaner.

* Fix typing
  • Loading branch information
mwaskom authored May 23, 2022
1 parent 651d06e commit 7d1c50f
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 9 deletions.
17 changes: 12 additions & 5 deletions seaborn/_core/plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -1042,22 +1042,29 @@ def _setup_scales(self, p: Plot, layers: list[Layer]) -> None:
axis = m["axis"]

prop = PROPERTIES.get(var if axis is None else axis, Property())
scale = self._get_scale(p, var, prop, var_values)
scale_spec = self._get_scale(p, var, prop, var_values)

# Initialize the data-dependent parameters of the scale
# Note that this returns a copy and does not mutate the original
# This dictionary is used by the semantic mappings
if scale is None:
if scale_spec is None:
# TODO what is the cleanest way to implement identity scale?
# We don't really need a ScaleSpec, and Identity() will be
# overloaded anyway (but maybe a general Identity object
# that can be used as Scale/Mark/Stat/Move?)
# Note that this may not be the right spacer to use
# (but that is only relevant for coordinates where identity scale
# doesn't make sense or is poorly defined — should it mean "pixes"?)
# (but that is only relevant for coordinates, where identity scale
# doesn't make sense or is poorly defined, since we don't use pixels.)
self._scales[var] = Scale([], lambda x: x, None, "identity", None)
else:
self._scales[var] = scale.setup(var_values, prop)
scale = scale_spec.setup(var_values, prop)
if isinstance(prop, Coordinate):
# If we have a coordinate here, we didn't assign a scale for it
# in _transform_coords, which means it was added during compute_stat
# This allows downstream orientation inference to work properly.
# But it feels a little hacky, so perhaps revisit.
scale.scale_type = "computed"
self._scales[var] = scale

def _plot_layer(self, p: Plot, layer: Layer) -> None:

Expand Down
4 changes: 2 additions & 2 deletions seaborn/_marks/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,10 @@ def _infer_orient(self, scales: dict) -> str: # TODO type scales
x_type = None if "x" not in scales else scales["x"].scale_type
y_type = None if "y" not in scales else scales["y"].scale_type

if x_type is None:
if x_type is None or x_type == "computed":
return "y"

elif y_type is None:
elif y_type is None or y_type == "computed":
return "x"

elif x_type != "nominal" and y_type == "nominal":
Expand Down
15 changes: 13 additions & 2 deletions seaborn/tests/_core/test_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -1350,20 +1350,31 @@ def test_orient_inference(self, long_df):

orient_list = []

class CaptureMoveOrient(Move):
class CaptureOrientMove(Move):
def __call__(self, data, groupby, orient):
orient_list.append(orient)
return data

(
Plot(long_df, x="x")
.pair(y=["b", "z"])
.add(MockMark(), move=CaptureMoveOrient())
.add(MockMark(), move=CaptureOrientMove())
.plot()
)

assert orient_list == ["y", "x"]

def test_computed_coordinate_orient_inference(self, long_df):

class MockComputeStat(Stat):
def __call__(self, df, groupby, orient, scales):
other = {"x": "y", "y": "x"}[orient]
return df.assign(**{other: df[orient] * 2})

m = MockMark()
Plot(long_df, y="y").add(m, MockComputeStat()).plot()
assert m.passed_orient == "y"

def test_two_variables_single_order_error(self, long_df):

p = Plot(long_df)
Expand Down

0 comments on commit 7d1c50f

Please sign in to comment.