Skip to content

Commit

Permalink
Pass through residual TODO comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mwaskom committed Jun 26, 2022
1 parent 69ae545 commit 91724ab
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 28 deletions.
45 changes: 17 additions & 28 deletions seaborn/_core/scales.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
Callable[[ArrayLike], ArrayLike], Callable[[ArrayLike], ArrayLike]
]

# TODO standardize String / ArrayLike interface
Pipeline = Sequence[Optional[Callable[[Union[Series, ArrayLike]], ArrayLike]]]


Expand Down Expand Up @@ -77,8 +76,7 @@ def _get_locators(self):
def _get_formatter(self):
raise NotImplementedError()

# TODO typing
def _get_scale(self, name, forward, inverse):
def _get_scale(self, name: str, forward: Callable, inverse: Callable):

major_locator, minor_locator = self._get_locators(**self._tick_params)
major_formatter = self._get_formatter(major_locator, **self._label_params)
Expand Down Expand Up @@ -156,7 +154,7 @@ def _setup(

units_seed = categorical_order(data, new.order)

# TODO move to new._get_scale?
# TODO move to Nominal._get_scale?
# TODO this needs some more complicated rethinking about how to pass
# a unit dictionary down to these methods, along with how much we want
# to invest in their API. What is it useful for tick() to do here?
Expand All @@ -169,7 +167,6 @@ def _setup(
# major_formatter = new._get_formatter(major_locator, **new._label_params)

class CatScale(mpl.scale.LinearScale):
# TODO turn this into a real thing I guess
name = None # To work around mpl<3.4 compat issues

def set_default_locators_and_formatters(self, axis):
Expand Down Expand Up @@ -356,7 +353,10 @@ def spacer(x):
return np.min(np.diff(np.sort(x.dropna().unique())))
new._spacer = spacer

# TODO make legend optional on per-plot basis with Scale parameter?
# TODO How to allow disabling of legend for all uses of property?
# Could add a Scale parameter, or perhaps Scale.suppress()?
# Are there other useful parameters that would be in Scale.legend()
# besides allowing Scale.legend(False)?
if prop.legend:
axis.set_view_interval(vmin, vmax)
locs = axis.major.locator()
Expand Down Expand Up @@ -397,8 +397,7 @@ def get_param(method, default):
elif arg == "sqrt":
return _make_sqrt_transforms()
else:
# TODO useful error message
raise ValueError()
raise ValueError(f"Unknown value provided for trans: {arg!r}")


@dataclass
Expand Down Expand Up @@ -544,8 +543,6 @@ def _get_locators(self, locator, at, upto, count, every, between, minor):
if locator is not None:
major_locator = locator

# TODO raise if locator is passed with any other parameters

elif upto is not None:
if log_base:
major_locator = LogLocator(base=log_base, numticks=upto)
Expand Down Expand Up @@ -744,15 +741,11 @@ def _get_formatter(self, locator, formatter, concise):
# ----------------------------------------------------------------------------------- #


class Calendric(Scale):
# TODO have this separate from Temporal or have Temporal(date=True) or similar?
...


class Binned(Scale):
# Needed? Or handle this at layer (in stat or as param, eg binning=)
...
# TODO Have this separate from Temporal or have Temporal(date=True) or similar?
# class Calendric(Scale):

# TODO Needed? Or handle this at layer (in stat or as param, eg binning=)
# class Binned(Scale):

# TODO any need for color-specific scales?
# class Sequential(Continuous):
Expand All @@ -773,7 +766,7 @@ class PseudoAxis:
code, this object acts like an Axis and can be used to scale other variables.
"""
axis_name = "" # TODO Needs real value? Just used for x/y logic in matplotlib
axis_name = "" # Matplotlib requirement but not actually used

def __init__(self, scale):

Expand All @@ -788,11 +781,9 @@ def __init__(self, scale):
self._data_interval = None, None

scale.set_default_locators_and_formatters(self)
# self.set_default_intervals() TODO mock?
# self.set_default_intervals() Is this ever needed?

def set_view_interval(self, vmin, vmax):
# TODO this gets called when setting DateTime units,
# but we may not need it to do anything
self._view_interval = vmin, vmax

def get_view_interval(self):
Expand All @@ -819,8 +810,6 @@ def set_major_locator(self, locator):
locator.set_axis(self)

def set_major_formatter(self, formatter):
# TODO matplotlib method does more handling (e.g. to set w/format str)
# We will probably handle that in the tick/format interface, though
self.major.formatter = formatter
formatter.set_axis(self)

Expand All @@ -846,12 +835,11 @@ def update_units(self, x):
if info is None:
return
if info.majloc is not None:
# TODO matplotlib method has more conditions here; are they needed?
self.set_major_locator(info.majloc)
if info.majfmt is not None:
self.set_major_formatter(info.majfmt)

# TODO this is in matplotlib method; do we need this?
# This is in matplotlib method; do we need this?
# self.set_default_intervals()

def convert_units(self, x):
Expand All @@ -863,10 +851,11 @@ def convert_units(self, x):
return self.converter.convert(x, self.units, self)

def get_scale(self):
# TODO matplotlib actually returns a string here!
# Note that matplotlib actually returns a string here!
# (e.g., with a log scale, axis.get_scale() returns "log")
# Currently we just hit it with minor ticks where it checks for
# scale == "log". I'm not sure how you'd actually use log-scale
# minor "ticks" in a legend context, so this is fine.....
# minor "ticks" in a legend context, so this is fine....
return self.scale

def get_majorticklocs(self):
Expand Down
6 changes: 6 additions & 0 deletions tests/_core/test_scales.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ def test_coordinate_transform_with_parameter(self, x):
s = Continuous(trans="pow3")._setup(x, Coordinate())
assert_series_equal(s(x), np.power(x, 3))

def test_coordinate_transform_error(self, x):

s = Continuous(trans="bad")
with pytest.raises(ValueError, match="Unknown value provided"):
s._setup(x, Coordinate())

def test_interval_defaults(self, x):

s = Continuous()._setup(x, IntervalProperty())
Expand Down

0 comments on commit 91724ab

Please sign in to comment.