-
-
Notifications
You must be signed in to change notification settings - Fork 798
Optional accessors in Sources #4277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
9ee0b6e
50a5c7d
80df283
8f826dd
590f1eb
930cf6a
bb18872
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| ListSource and TreeSource now allow mapping-based data to be used without explicitly providing accessors. |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -18,7 +18,7 @@ | |||||||||
| def _find_item( | ||||||||||
| candidates: Sequence[T], | ||||||||||
| data: object, | ||||||||||
| accessors: Sequence[str], | ||||||||||
| accessors: Sequence[str] | None, | ||||||||||
| start: T | None, | ||||||||||
| error: str, | ||||||||||
| ) -> T: | ||||||||||
|
|
@@ -29,6 +29,9 @@ def _find_item( | |||||||||
| else: | ||||||||||
| start_index = 0 | ||||||||||
|
|
||||||||||
| if accessors is None and not isinstance(data, Mapping): | ||||||||||
| raise ValueError("Cannot search for non-mapping data without accessors") | ||||||||||
|
|
||||||||||
| for item in candidates[start_index:]: | ||||||||||
| try: | ||||||||||
| if isinstance(data, Mapping): | ||||||||||
|
|
@@ -103,23 +106,28 @@ def __delattr__(self, attr: str) -> None: | |||||||||
|
|
||||||||||
| class ListSource(Source): | ||||||||||
| _data: list[Row] | ||||||||||
| _accessors: list[str] | None | ||||||||||
|
|
||||||||||
| def __init__(self, accessors: Iterable[str], data: Iterable | None = None): | ||||||||||
| def __init__( | ||||||||||
| self, accessors: Iterable[str] | None = None, data: Iterable | None = None | ||||||||||
| ): | ||||||||||
| """A data source to store an ordered list of multiple data values. | ||||||||||
|
|
||||||||||
| :param accessors: A list of attribute names for accessing the value | ||||||||||
| in each column of the row. | ||||||||||
| in each column of the row. If omitted, only mapping row data can be | ||||||||||
| converted. | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| :param data: The initial list of items in the source. Items are converted as | ||||||||||
| shown [above][listsource-item]. | ||||||||||
| """ | ||||||||||
| super().__init__() | ||||||||||
| if isinstance(accessors, str) or not hasattr(accessors, "__iter__"): | ||||||||||
| if accessors is None: | ||||||||||
| self._accessors = None | ||||||||||
| elif isinstance(accessors, str) or not hasattr(accessors, "__iter__"): | ||||||||||
| raise ValueError("accessors should be a list of attribute names") | ||||||||||
|
|
||||||||||
| # Copy the list of accessors | ||||||||||
| self._accessors = list(accessors) | ||||||||||
| if len(self._accessors) == 0: | ||||||||||
| raise ValueError("ListSource must be provided a list of accessors") | ||||||||||
| else: | ||||||||||
| # Copy the list of accessors | ||||||||||
| accessors = list(accessors) | ||||||||||
| self._accessors = accessors if len(accessors) > 0 else None | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's an edge case; but for consistency, I'd argue an empty list should be interpreted as a list of no accessors, rather than "accessors haven't been provided". That would technically allow construction of "empty" Row objects from list-like data. That isn't really useful, but it's internally consistent (and the implementation is simpler as well).
Suggested change
|
||||||||||
|
|
||||||||||
| # Convert the data into row objects | ||||||||||
| if data is not None: | ||||||||||
|
|
@@ -128,8 +136,10 @@ def __init__(self, accessors: Iterable[str], data: Iterable | None = None): | |||||||||
| self._data = [] | ||||||||||
|
|
||||||||||
| @property | ||||||||||
| def accessors(self) -> list[str]: | ||||||||||
| def accessors(self) -> list[str] | None: | ||||||||||
| """The attribute names for accessing the value in each column of a row.""" | ||||||||||
| if self._accessors is None: | ||||||||||
| return None | ||||||||||
| return self._accessors.copy() | ||||||||||
|
|
||||||||||
| ###################################################################### | ||||||||||
|
|
@@ -158,10 +168,13 @@ def __delitem__(self, index: int) -> None: | |||||||||
| def _create_row(self, data: object) -> Row: | ||||||||||
| if isinstance(data, Mapping): | ||||||||||
| row = Row(**data) | ||||||||||
| elif hasattr(data, "__iter__") and not isinstance(data, str): | ||||||||||
| row = Row(**dict(zip(self._accessors, data, strict=False))) | ||||||||||
| elif self._accessors is not None: | ||||||||||
| if hasattr(data, "__iter__") and not isinstance(data, str): | ||||||||||
| row = Row(**dict(zip(self._accessors, data, strict=False))) | ||||||||||
| else: | ||||||||||
| row = Row(**{self._accessors[0]: data}) | ||||||||||
| else: | ||||||||||
| row = Row(**{self._accessors[0]: data}) | ||||||||||
| raise ValueError("ListSource requires accessors for non-mapping row data") | ||||||||||
| row._source = self | ||||||||||
| return row | ||||||||||
|
|
||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -204,25 +204,31 @@ def find(self, data: object, start: Node[T] | None = None) -> Node[T]: | |
|
|
||
| class TreeSource(Source): | ||
| _roots: list[Node] | ||
| _accessors: list[str] | None | ||
|
|
||
| def __init__(self, accessors: Iterable[str], data: object | None = None): | ||
| def __init__( | ||
| self, accessors: Iterable[str] | None = None, data: object | None = None | ||
| ): | ||
| super().__init__() | ||
| if isinstance(accessors, str) or not hasattr(accessors, "__iter__"): | ||
| if accessors is None: | ||
| self._accessors = None | ||
| elif isinstance(accessors, str) or not hasattr(accessors, "__iter__"): | ||
| raise ValueError("accessors should be a list of attribute names") | ||
|
|
||
| # Copy the list of accessors | ||
| self._accessors = list(accessors) | ||
| if len(self._accessors) == 0: | ||
| raise ValueError("TreeSource must be provided a list of accessors") | ||
| else: | ||
| # Copy the list of accessors, in case of [] its None. | ||
| accessors = list(accessors) | ||
| self._accessors = accessors if len(accessors) > 0 else None | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As with list source; this can be simplified for consistency. |
||
|
|
||
| if data is not None: | ||
| self._roots = self._create_nodes(parent=None, value=data) | ||
| else: | ||
| self._roots = [] | ||
|
|
||
| @property | ||
| def accessors(self) -> list[str]: | ||
| def accessors(self) -> list[str] | None: | ||
| """The attribute names for accessing the value in each column of a row.""" | ||
| if self._accessors is None: | ||
| return None | ||
| return self._accessors.copy() | ||
|
|
||
| ###################################################################### | ||
|
|
@@ -253,10 +259,13 @@ def _create_node( | |
| ) -> Node: | ||
| if isinstance(data, Mapping): | ||
| node = Node(**data) | ||
| elif hasattr(data, "__iter__") and not isinstance(data, str): | ||
| node = Node(**dict(zip(self._accessors, data, strict=False))) | ||
| elif self._accessors is not None: | ||
| if hasattr(data, "__iter__") and not isinstance(data, str): | ||
| node = Node(**dict(zip(self._accessors, data, strict=False))) | ||
| else: | ||
| node = Node(**{self._accessors[0]: data}) | ||
| else: | ||
| node = Node(**{self._accessors[0]: data}) | ||
| raise ValueError("TreeSource requires accessors for non-mapping node data") | ||
|
|
||
| node._parent = parent | ||
| node._source = self | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |
|
|
||
| Data sources are abstractions that allow you to define the data being managed by your application independent of the GUI representation of that data. For details on the use of data sources, see the [topic guide](/topics/data-sources.md). | ||
|
|
||
| ListSource is an implementation of an ordered list of data. When a ListSource is created, it is given a list of `accessors` - these are the attributes that all items managed by the ListSource will have. The API provided by ListSource is [`list`][]-like; the operations you'd expect on a normal Python list, such as `insert`, `remove`, `index`, and indexing with `[]`, are also possible on a ListSource: | ||
| ListSource is an implementation of an ordered list of data. When a ListSource is created, it is given a list of `accessors` - these are the attributes that all items managed by the ListSource will have. If no accessors are provided, or an empty sequence is given, the source stores `None`. The API provided by ListSource is [`list`][]-like; the operations you'd expect on a normal Python list, such as `insert`, `remove`, `index`, and indexing with `[]`, are also possible on a ListSource: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an accurate description of the implementation, but that doesn't describe how or why you'd use a non-accessor ListSource. The docs should be focussed on usage. |
||
|
|
||
| ```python | ||
| from toga.sources import ListSource | ||
|
|
@@ -34,7 +34,7 @@ source.insert(0, {"name": "Bettong", "weight": 1.2}) | |
|
|
||
| [](){ #listsource-item } | ||
|
|
||
| The ListSource manages a list of [`Row`][toga.sources.Row] objects. Each Row has all the attributes described by the source's `accessors`. A Row object will be constructed for each item that is added to the ListSource, and each item can be: | ||
| The ListSource manages a list of [`Row`][toga.sources.Row] objects. Each Row has all the attributes described by the source's `accessors`. If accessors are omitted, or an empty sequence is provided, `accessors` will be `None`. A Row object will be constructed for each item that is added to the ListSource, and each item can be: | ||
|
|
||
| - A dictionary, with the accessors mapping to the keys in the dictionary. | ||
| - Any other iterable object (except for a string), with the accessors being mapped onto the items in the iterable in order of definition. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor code style issue - we try to avoid the "all args on one separate line" format, even though it's legal from Ruff's perspective. We prefer to go directly to "one arg per line":