Add list_property, and other property cleanups

[HalfWhitt]

I'm still working on composite_property, but there's enough going on here I decided to split it into separate PRs.

First of all, I've made some API decisions that I'm open to being talked out of:

• The property returns a tuple rather than a list. It semantically makes more sense as a list, but returning a mutable object from a validated property is dangerous. The unwary could call style.series_prop.append("something") and completely bypass validation.
• To avoid misleading about the return type, I've renamed it from list_property to series_property. Could also be... sequence? Something else?
• I've treated assigning an empty sequence the same as assigning None: it raises an error and suggests using del to unset the property.
• Duplicate values are filtered out during assignment. I don't know of a reason not to do this, but it's conceivable there might be some edge case somewhere...

I've also made some other tweaks to the existing code:

• I accidentally left unused arguments in directional_property, from when I was giving it too many brains. They're now removed.
• I went through BaseStyle's internal methods and noticed several I could simplify, particularly from using in now that it's defined.
• I used __init_subclass__ to give each style subclass a direct reference to its set of properties, so it doesn't have to index into the overall dict over and over again. I like how it makes things cleaner, but I'll understand and undo if you feel like this is a bit too much fiddly magic. (It would be cool if this could just replace the dict, but __init_subclass__ isn't called until after all the properties are initialized and named.)

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

[HalfWhitt]

Somewhat related question: the dict-like __set__ and __get__ methods currently only work on "actual" properties, not aliases in _ALL_PROPERTIES. Is that an intentional limitation, or should I expand that?

[freakboy3742]

I'm still working on composite_property, but there's enough going on here I decided to split it into separate PRs.

First of all, I've made some API decisions that I'm open to being talked out of:

• The property returns a tuple rather than a list. It semantically makes more sense as a list, but returning a mutable object from a validated property is dangerous. The unwary could call style.series_prop.append("something") and completely bypass validation.

A tuple isn't "just an immutable list"; there are other semantic details that are significant. If we need a "readonly list", then we should create one by creating a custom class that allows list-like access, but doesn't expose modification properties (or exposes them in a way that does trigger validation - but readonly seems like a much easier option).

• To avoid misleading about the return type, I've renamed it from list_property to series_property. Could also be... sequence? Something else?

I think this becomes a moot point once we're using a readonly-list for storage.

• I've treated assigning an empty sequence the same as assigning None: it raises an error and suggests using del to unset the property.

That makes sense to me.

• Duplicate values are filtered out during assignment. I don't know of a reason not to do this, but it's conceivable there might be some edge case somewhere...

I appreciate that in the font case it's redundant, but is there a technical reason to do this deduplication? I imagine it's going to be more effort to implement deduplication, and the worst case in the font usage is "you try to load a bad font twice"... which... is exactly what you asked for. Is there a technical reason to do this? If there isn't, I'd rather stick to a "dumb" implementation than try and presuppose future usage.

If we are going to remove duplicates, we need to ensure that "first usage" order is preserved - [a b a c] should reduce to [a b c], not [b a c].

I've also made some other tweaks to the existing code:

These all make sense to me.

[HalfWhitt]

A tuple isn't "just an immutable list"; there are other semantic details that are significant. If we need a "readonly list", then we should create one by creating a custom class that allows list-like access, but doesn't expose modification properties (or exposes them in a way that does trigger validation - but readonly seems like a much easier option).

I realize that lists are customarily used for arbitrary-length collections of more-or-less homogeneous items, and tuples for fixed-length collections where position matters... but even the Python docs say "Tuples are also used for cases where an immutable sequence of homogeneous data is needed (such as allowing storage in a set." I'll subclass UserList and overwrite all fifteen mutable methods if you really do prefer it, it just seems like reinventing a wheel.

(I did think of the idea of a self-validating list, but I have a hard time imagining it wouldn't be more trouble than it would be worth.)

I appreciate that in the font case it's redundant, but is there a technical reason to do this deduplication? I imagine it's going to be more effort to implement deduplication, and the worst case in the font usage is "you try to load a bad font twice"... which... is exactly what you asked for. Is there a technical reason to do this? If there isn't, I'd rather stick to a "dumb" implementation than try and presuppose future usage.

The de-duplication is already preserving order, but no, I guess there's no real reason we need it. I'll yank it out.

Any opinion on the __get__/__set__ methods? You were probably already typing your last comment when I posted mine.

[freakboy3742]

A tuple isn't "just an immutable list"; there are other semantic details that are significant. If we need a "readonly list", then we should create one by creating a custom class that allows list-like access, but doesn't expose modification properties (or exposes them in a way that does trigger validation - but readonly seems like a much easier option).

I realize that lists are customarily used for arbitrary-length collections of more-or-less homogeneous items, and tuples for fixed-length collections where position matters... but even the Python docs say "Tuples are also used for cases where an immutable sequence of homogeneous data is needed (such as allowing storage in a set." I'll subclass UserList and overwrite all fifteen mutable methods if you really do prefer it, it just seems like reinventing a wheel.

Reasonable people can disagree with the Python docs :-P

As for subclassing - It sounds like a proxy API might be easier. I can't remember if this was a PR from you or someone else, but subclassing a primitive type means you can accidentally inherit capabilities (sometimes with hillarious consequences [see footnote 1]) - we only really need __getitem__, __len__, __iter__, __next__, __str__ and __repr__... writing a class that wraps an underlying list, but only exposes these methods to the underlying implementations seems like a lot less effort, and less prone to potential issues.

(I did think of the idea of a self-validating list, but I have a hard time imagining it wouldn't be more trouble than it would be worth.)

Yeah - we can shave that yak if/when it starts looking necessary.

Any opinion on the __get__/__set__ methods? You were probably already typing your last comment when I posted mine.

It wasn't a deliberate choice AFAIR. If we can expand the usage to work with all aliases, I can't think of any reason not to.

[1] I've actually worked on the system described in that anecdote (JSAF), and... I don't entirely believe the explanation given by the DSTO employee. I don't remember "beachballs" being a default anything in JSAF, and I accidentally made an aircraft carrier travel at mach 2 because of a subclassing error. Object LISP: not even once, my friends :-)

[HalfWhitt]

I think this may legit be the first time I've ever found myself on this side of a disagreement over technical correctness vs. pragmatism. It's a strange experience. 🤣

[...]we only really need __getitem__, __len__, __iter__, __next__, __str__ and __repr__...

I added __eq__ so it can be compared against regular lists as well as itself, but left out __next__ to match regular lists since they're not iterators.

Reasonable people can disagree with the Python docs :-P

Don't worry, I won't tell the Inquisitors.

[...] I accidentally made an aircraft carrier travel at mach 2 because of a subclassing error.

Make that happen by accident in Toga and then I'll really be impressed. 😉

[freakboy3742]

Definitely on the right track; a couple of minor issues flagged inline.

[freakboy3742]

Ok - a couple of really minor cosmetic suggestions, and I think we're there!

[freakboy3742]

Ok - two last minute issues I picked up, but this is good to go! Thanks for the PR!