Skip to content

Commit

Permalink
Consistently handle style properties that are explicitly set to initi…
Browse files Browse the repository at this point in the history
…al value (beeware#3151)

Differentiates between unset values, and values that have been explicitly set to their initial value.
  • Loading branch information
HalfWhitt authored Feb 4, 2025
1 parent 71a5826 commit 8e222c2
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 57 deletions.
1 change: 1 addition & 0 deletions changes/3151.misc.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Style properties are now consistently counted as having been set, even when they are explicitly set to their default initial value.
3 changes: 3 additions & 0 deletions core/src/toga/compat/collections/abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ def __init__(self, abc_cls):
def __getitem__(self, key):
return self.abc_cls

def register(self, cls):
pass


for cls_name in __all__:
globals()[cls_name] = _SubscriptWrapper(type(cls_name, (object,), {}))
129 changes: 72 additions & 57 deletions travertino/src/travertino/declaration.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,18 @@ def __str__(self):
def __repr__(self):
return repr(self._data)

def __reversed__(self):
return reversed(self._data)

def index(self, value):
return self._data.index(value)

def count(self, value):
return self._data.count(value)


Sequence.register(ImmutableList)


class Choices:
"A class to define allowable data types for a property"
Expand Down Expand Up @@ -108,24 +120,13 @@ def __init__(
:param integer: Are integers allowed as values?
:param number: Are numbers allowed as values?
:param color: Are colors allowed as values?
:param initial: The initial value for the property.
:param initial: The initial value for the property. If the property has not been
explicitly set, this is what is returned when it's accessed.
"""
self.choices = Choices(
*constants, string=string, integer=integer, number=number, color=color
)
self.initial = None

try:
# If an initial value has been provided, it must be consistent with
# the choices specified.
if initial is not None:
self.initial = self.validate(initial)
except ValueError:
# Unfortunately, __set_name__ hasn't been called yet, so we don't know the
# property's name.
raise ValueError(
f"Invalid initial value {initial!r}. Available choices: {self.choices}"
)
self.initial = None if initial is None else self.validate(initial)

def __set_name__(self, owner, name):
self.name = name
Expand Down Expand Up @@ -153,7 +154,15 @@ def __set__(self, obj, value):

value = self.validate(value)

if value != getattr(obj, f"_{self.name}", self.initial):
if (current := getattr(obj, f"_{self.name}", None)) is None:
# If the value has not been explicitly set already, then we always want to
# assign to the attribute -- even if the value being assigned is identical
# to the initial value.
setattr(obj, f"_{self.name}", value)
if value != self.initial:
obj.apply(self.name, value)

elif value != current:
setattr(obj, f"_{self.name}", value)
obj.apply(self.name, value)

Expand All @@ -166,8 +175,8 @@ def __delete__(self, obj):
obj.apply(self.name, self.initial)

@property
def _name_if_set(self, default=""):
return f" {self.name}" if hasattr(self, "name") else default
def _name_if_set(self):
return f" {self.name}" if hasattr(self, "name") else ""

def validate(self, value):
try:
Expand Down Expand Up @@ -230,20 +239,19 @@ def __init__(self, name_format):
:param name_format: The format from which to generate subproperties. "{}" will
be replaced with "_top", etc.
"""
self.name_format = name_format
self.property_names = [
name_format.format(f"_{direction}") for direction in self.DIRECTIONS
]

def __set_name__(self, owner, name):
self.name = name
owner._BASE_ALL_PROPERTIES[owner].add(self.name)

def format(self, direction):
return self.name_format.format(f"_{direction}")

def __get__(self, obj, objtype=None):
if obj is None:
return self

return tuple(obj[self.format(direction)] for direction in self.DIRECTIONS)
return tuple(obj[name] for name in self.property_names)

def __set__(self, obj, value):
if value is self:
Expand All @@ -255,22 +263,20 @@ def __set__(self, obj, value):
value = (value,)

if order := self.ASSIGNMENT_SCHEMES.get(len(value)):
for direction, index in zip(self.DIRECTIONS, order):
obj[self.format(direction)] = value[index]
for name, index in zip(self.property_names, order):
obj[name] = value[index]
else:
raise ValueError(
f"Invalid value for '{self.name}'; value must be a number, or a 1-4 "
f"tuple."
)

def __delete__(self, obj):
for direction in self.DIRECTIONS:
del obj[self.format(direction)]
for name in self.property_names:
del obj[name]

def is_set_on(self, obj):
return any(
hasattr(obj, self.format(direction)) for direction in self.DIRECTIONS
)
return any(hasattr(obj, name) for name in self.property_names)


class BaseStyle:
Expand All @@ -297,8 +303,8 @@ def _ALL_PROPERTIES(self):

# Fallback in case subclass isn't decorated as subclass (probably from using
# previous API) or for pre-3.10, before kw_only argument existed.
def __init__(self, **style):
self.update(**style)
def __init__(self, **properties):
self.update(**properties)

@property
def _applicator(self):
Expand All @@ -324,6 +330,34 @@ def _applicator(self, value):
stacklevel=2,
)

def reapply(self):
for name in self._PROPERTIES:
self.apply(name, self[name])

def copy(self, applicator=None):
"""Create a duplicate of this style declaration."""
dup = self.__class__()
dup.update(**self)

######################################################################
# 10-2024: Backwards compatibility for Toga <= 0.4.8
######################################################################

if applicator is not None:
warn(
"Providing an applicator to BaseStyle.copy() is deprecated. Set "
"applicator afterward on the returned copy.",
DeprecationWarning,
stacklevel=2,
)
dup._applicator = applicator

######################################################################
# End backwards compatibility
######################################################################

return dup

######################################################################
# Interface that style declarations must define
######################################################################
Expand All @@ -342,35 +376,15 @@ def layout(self, viewport):
# Provide a dict-like interface
######################################################################

def reapply(self):
for name in self._PROPERTIES:
self.apply(name, self[name])

def update(self, **styles):
"Set multiple styles on the style definition."
for name, value in styles.items():
def update(self, **properties):
"""Set multiple styles on the style definition."""
for name, value in properties.items():
name = name.replace("-", "_")
if name not in self._ALL_PROPERTIES:
raise NameError(f"Unknown style '{name}'")

self[name] = value

def copy(self, applicator=None):
"Create a duplicate of this style declaration."
dup = self.__class__()
dup.update(**self)

if applicator is not None:
warn(
"Providing an applicator to BaseStyle.copy() is deprecated. Set "
"applicator afterward on the returned copy.",
DeprecationWarning,
stacklevel=2,
)
dup._applicator = applicator

return dup

def __getitem__(self, name):
name = name.replace("-", "_")
if name in self._ALL_PROPERTIES:
Expand All @@ -392,13 +406,13 @@ def __delitem__(self, name):
raise KeyError(name)

def keys(self):
return {name for name in self._PROPERTIES if name in self}
return {name for name in self}

def items(self):
return [(name, self[name]) for name in self._PROPERTIES if name in self]
return [(name, self[name]) for name in self]

def __len__(self):
return sum(1 for name in self._PROPERTIES if name in self)
return sum(1 for _ in self)

def __contains__(self, name):
return name in self._ALL_PROPERTIES and (
Expand Down Expand Up @@ -432,6 +446,7 @@ def __ior__(self, other):
######################################################################
# Get the rendered form of the style declaration
######################################################################

def __str__(self):
return "; ".join(
f"{name.replace('_', '-')}: {value}" for name, value in sorted(self.items())
Expand Down
36 changes: 36 additions & 0 deletions travertino/tests/test_declaration.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

from collections.abc import Sequence
from unittest.mock import call
from warnings import catch_warnings, filterwarnings

Expand Down Expand Up @@ -606,6 +607,14 @@ def test_list_property_list_like():
count += 1
assert count == 4

assert [*reversed(prop)] == [VALUE2, 3, 2, 1]

assert prop.index(3) == 2

assert prop.count(VALUE2) == 1

assert isinstance(prop, Sequence)


@pytest.mark.parametrize("StyleClass", [Style, DeprecatedStyle])
def test_set_multiple_properties(StyleClass):
Expand Down Expand Up @@ -754,6 +763,33 @@ def test_dict(StyleClass):
del style["no-such-property"]


@pytest.mark.parametrize("StyleClass", [Style, DeprecatedStyle])
def test_set_to_initial(StyleClass):
"""A property set to its initial value is distinct from an unset property."""
style = StyleClass()

# explicit_const's initial value is VALUE1.
assert style.explicit_const == VALUE1
assert "explicit_const" not in style

# The unset property shouldn't affect the value when overlaid over a style with
# that property set.
non_initial_style = StyleClass(explicit_const=VALUE2)
union = non_initial_style | style
assert union.explicit_const == VALUE2
assert "explicit_const" in union

# The property should count as set, even when set to the same initial value.
style.explicit_const = VALUE1
assert style.explicit_const == VALUE1
assert "explicit_const" in style

# The property should now overwrite.
union = non_initial_style | style
assert union.explicit_const == VALUE1
assert "explicit_const" in union


@pytest.mark.parametrize("StyleClass", [Style, DeprecatedStyle])
@pytest.mark.parametrize("instantiate", [True, False])
def test_union_operators(StyleClass, instantiate):
Expand Down

0 comments on commit 8e222c2

Please sign in to comment.