Design: Minimum Viable Table and Tree Columns

[corranwebster]

What is the problem or limitation you are having?

The logic for how Table and Tree widgets extract text, icons, etc. from a row is somewhat complex, and is scattered around and replicated in a number of places. The idea of this proposal is to gather as much of that logic as possible into one place, and to do so in a way which is backward compatible, but also gives a framework for extending to allow other approaches and UI elements in the future.

In previous issues and PRs, there has been discussion of the idea of a Column object, that would satisfy this, but they appear to be outdated.

Rel #1066, #1478, #3760 .

Describe the solution you'd like

The proposal is to replace the current lists of headings and accessors in the Table and Tree widgets with a list of Column objects.

The first step would be to simply have the Columns store the values of each heading and accessor and effectively be an implementation detail: the list of columns would be the source of truth for the headers and accessors, and the current headings and accessors properties would extract the values from the Column list dynamically. The columns would be responsible for converting headings to accessors, rather than have that logic in the Table and Tree, and the insert/remove column methods would be adapted to insert/remove Column objects, which would also allow simplification of those methods. This also unifies a lot of the heading/accessor handling into a common location, rather than duplicating in the Table and Tree

This would look something like this (maybe with heading and accessor as properties):

class AccessorColumn:
    heading: str
    accessor: str

    def __init__(
        self,
        heading: str | None = None,
        accessor: str | None = None,
    ):
        if accessor is None:
            if heading is not None:
                accessor = to_accessor(heading)
            else:
                raise ValueError(
                    "Cannot create a column without either headings or accessors"
                )
        self.heading = heading
        self.accessor = accessor

The second step would be to add value, text, icon (and possibly widget) methods to this class which take a row, and use the rules for getting text and icons out of a row, something like:

def value(self, row):
    return getattr(row, self.accessor, None)

def text(self, row):
    value = self.value(row)
    if isinstance(value, tuple):
        value = value[1]
    if value is not None:
        value = str(value)
    return value

def icon(self, row):
    value = self.value(row)
    if isinstance(value, tuple):
        value = value[0]
    elif hasattr(value, 'icon'):
        value = value.icon
    return value

With this in place the widget implementations can be adapted so that instead of implementing this logic internally, they instead ask the appropriate column object for the text or icon to display for a given row. Similarly, they can ask a column for its heading. The goal is to remove direct use of accessors in the implementations. The ListSource and TreeSource will still need them, but the widget implementations should consider accessors to be a data source implementation detail.

This refactoring then opens up the API for piecemeal improvements, such as:

• extending the Column API to expose other information which might be needed to display the column (tool-tips, checkmarks, colors, fonts, etc.) or the ways the information might be extracted from values (eg. if the value is a mapping, look for text and icon keys).
• deprecating passing headings and accessors and instead passing Column objects
• making the Column's methods a protocol which can be implemented in other ways (eg. columns which compute values from the rows in various ways) or which are tied closely to a particular Source implementation.
• adding the ability to flag the column cells editable and set the value of the accessor on the row.
• proper embedding of widgets or their analogues on non-macOS platforms
• and probably lots of other things besides

All of these improvements are hypothetical, and not part of this proposal. Rather this is intended to be a minimal, backward-compatible refactoring that creates the Column objects and unifies the accessor logic so future changes can be made in one place and alternatives can be explored.

Describe alternatives you've considered

The status-quo works, so this is not urgent or required. It's possible that it might be better to revive a PR like #1478 or to instead design a better end-state and build it directly and not incrementally.

A more minimal API without the intent to head in the direction of #1478 would be a common utility function which contains the logic for extracting values: give it a row and an accessor and it returns the widget, text and icon, possibly also handling missing values and warning for backends which don't support Widgets.

This is making Tables and Trees more "column-oriented" which can impede transposed views (eg. comparisons of objects where each column is an object, and each row is a feature or statistic of that object) or grid/spreadhseet views (eg. a display of a 2D array of numbers).

The Qt approach where instead of a bunch of distinct column objects, you have the source object grow methods for supplying text, icons, etc. may possibly be better.

Additional context

There is an implementation of these ideas in #4042.

[freakboy3742]

Completely agreed with the overall motivation here - there's a lot of common column-related logic that could/should be implemented in core, and a "Column" abstraction makes sense in that context.

The only thing that gives me pause around Column specifically as a name is the overlap with DetailedList - a DetailedList has a lot of very similar sorts of logic around accessors for icons, primary and secondary text. However, I think that might be just far enough off that it's a separate concept.

You've already noted the overlap with #1478; there's some questions there about where style ends and widget definition begins. I'm not 100% sure if the approach in that PR of making a Column a full "Widget" makes sense; the sort of approach you're describing here where Column is a distinct object that is closer to "OptionItem" than a standalone Widget strikes me as a more natural approach. Longer term, it provides a hook where a style property could be set, allowing control of column weighting, font, color etc.

I'm not overly familiar with Qt's API - but it's obviously a good place for comparisons. I'm not sure that the source is the right place for this sort of abstraction though. To me, a source is "pure data", and a widget is a view over that data. Two different Tables could view the same source, displaying different icons, columns, etc. If the visualisation mechanism is tied to the Source, I'm not sure how you'd manifest that sort of display. That said - there is definitely potential for better ways to define/extract icons for columns - the current "return an (icon, text) tuple works, but is a little cryptic; a full Column interface provides a way to define exactly where the icon will come from.

[corranwebster]

The only thing that gives me pause around Column specifically as a name is the overlap with DetailedList - a DetailedList has a lot of very similar sorts of logic around accessors for icons, primary and secondary text. However, I think that might be just far enough off that it's a separate concept.

My gut feeling is that both Column, OptionItem and whatever DetailedList is are three examples of an "adapt a data source to things the UI can use" pattern. I can also see something like that for a pure ValueSource: an adapter that takes the value in a ValueSource and returns the formatted text and icon (eg. if value doesn't validate) to display in a TextInput

[freakboy3742]

The only thing that gives me pause around Column specifically as a name is the overlap with DetailedList - a DetailedList has a lot of very similar sorts of logic around accessors for icons, primary and secondary text. However, I think that might be just far enough off that it's a separate concept.

My gut feeling is that both Column, OptionItem and whatever DetailedList is are three examples of an "adapt a data source to things the UI can use" pattern. I can also see something like that for a pure ValueSource: an adapter that takes the value in a ValueSource and returns the formatted text and icon (eg. if value doesn't validate) to display in a TextInput

OptionItem is a slightly different case IMHO, because it requires a complex GUI layout, not just data. However, I accept the idea that DetailedList is really defining a "data source adapter" protocol, and it makes some sense for that protocol to be widget-specific - although we should try to maintain a consistent "flavour", and reuse any pieces where re-use makes sense.

The complication for ValueSource is a little deeper though. Table and Tree widgets are usually implemented with a "view of data" model baked in, so the idea of a single Source with multiple Table widgets looking at that source works without too much difficulty. But if I have a single ValueSource with two TextInputs pointed at it, I essentially have three copies of the underlying text. So which one is canonical? And how do we ensure that the copies in the actual widgets - which are the ones that are functionally most effective - are kept in alignment with the ValueSource?