Add union operators to BaseStyle

[HalfWhitt]

I thought it would be nice to have | and |= operators as part of BaseStyle's "dict-like interface", and while adding them I noticed that it already supported most of the MutableMapping abstract base class. All we were missing were __iter__ and __len__!

Explicitly inheriting from the abc theoretically gives us free implementations of __contains__, keys, and items, although __contains__ and __keys__ need to be redefined because the automatic implementation apparently pulls in all fields on the dataclass. The inherited items works fine though.

I've set up the union operators such that any mapping can be layered onto a style except a style of a different class, since each class has its own explicitly separate set of properties. Of course, a dictionary or other mapping with invalid keys or values for the style will still raise an error.

P.S. Those two little operators were the entire reason I started looking at declaration, or indeed even at Travertino, in the first place. Damn yaks! 😉

PR Checklist:

• All new features have been tested
• All new features have been documented
• I have read the CONTRIBUTING.md file
• I will abide by the code of conduct

[freakboy3742]

The implementation and test looks good; and I don't have any objection to adding support for the or operator, or filling out other dunder methods that might be helpful.

However, introducing a base classes makes me a little nervous because we're inheriting a bunch of behavior - and we need to make sure that behavior doesn't add something we don't want.

The one that stands out for me is clear() - subclassing MutableMapping adds a clear() implementation, but it's not clear to me that this implementation will have the side effects that we need it to have (in particular, triggering an apply()). The same is likely true of pop(), popitem() and setdefault().

I think we're overriding all the other methods that MutableMapping provides (e.g., update()), so we're not actually gaining any benefit from subclassing MutableMapping for those methods.

So - is there anything specific that we gain from subclassing in this instance? Could we just add the __or__() implementations and leave it at that?

[HalfWhitt]

Ah, fair point. If all else were equal, it's kind of nice to have an explicit declaration that this behaves as a MutableMapping, and it enforces the presence of the required methods. But I hadn't thought of those ways things aren't equal, and I believe the only concrete thing we're currently getting "for free" is items, which was previously there anyway.

isinstance(style, MutableMapping) won't be True anymore, but I can rearrange the type checks in the operators.

[HalfWhitt]

Ah, we were also getting get as a mixin. It's simple enough to implement, though now that I think about it, I wonder how useful it is, at least with the "default default" being None, since attributes with no initial or set value also return that. I suppose there's probably not a real need for it anyway.

[freakboy3742]

It's always the little changes that are hard to nail down; a couple of questions inline.

[freakboy3742]

This is an interesting case... Sure, implicit hasn't been defined... but it does exist, and it does have a value if you try to retrieve it. I'd be inclined to say implicit in style is True, not False.

I guess the interesting cases missing in this test are explicit_none and thing. Is the in operator reporting if the style parameter exists, or if the style parameter has been explicitly set?

[HalfWhitt]

From a usability standpoint, I feel like being able to check if it's set is more useful. Presumably you define your style class once, so I would expect that 99% of the time you're interested in testing what keys are "in" a given style object, it's the ones currently set on that object. This is consistent with what the keys method already did.

That said, I hadn't actually considered how weird directional properties are. If you set the top padding, one would then expect"padding" in style to be true... even though it's not actually a "key" on its own.

[HalfWhitt]

What are you thinking of about the explicit_none case that's different from implicit?

[HalfWhitt]

I've made the in tests more comprehensive; I've also, at least for now, kept its "only properties that are set" behavior, but expanded it to return True for a directional property if at least one of its aliased properties is set.

[freakboy3742]

That's a fair point - "have I set this value" is more likely to be the question asked than "is this key valid" (not that either seem especially likely). Plus, I guess we have hasattr() to confirm attribute existence.

As for the explicit_none case - I guess there isn't really a difference with implicit (unless there's a distinction between "has a value at all" and "has had a value explicitly set", which is admittedly even more nebulous than the "property exists" vs "has been set" distinction). I was only drawing attention to it as an exemplar of whether in is confirming whether the attribute has a value, or whether the value has been set, but isn't dependent on implicit defaults.

[HalfWhitt]

Hm... If I'm not mistaken, once they're constructed in the class definition, there's no functional difference between the implicit and explicit_none properties. Neither of them sets an underscore attribute on a style instance unless a value is assigned to it, and neither initially counts as being "set"; a property can never "actually be" None, so that it unambiguously means "not set, with no initial to fall back to".

[HalfWhitt]

...There are too many kinds of nothing. 😵‍💫

[freakboy3742]

All looks good to me - glad you could finally get to the yak you actually wanted to shave :-)