Create table from query with multiple columns and introduce tiles #150
Conversation
|
✅ 29/29 passed, 2 skipped, 7m11s total Running from acceptance #175 |
| widget = Widget(name=dataset.name, queries=[named_query], spec=counter_spec) | ||
| spec: WidgetSpec | ||
| if len(fields) == 1: # Counters are expected to have one field | ||
| counter_encodings = CounterFieldEncoding(field_name=fields[0].name, display_name=fields[0].name) |
There was a problem hiding this comment.
can you refactor this code to introduce three new classes: CounterTile, TableTile, and MarkdownTile. Those classes should have a widget_spec() -> WidgetSpec and size() -> tuple[int, int] methods.
There was a problem hiding this comment.
Sure, do you want each of them to receive the fields on initialisation? And the WidgetMetdata, how should that be merged with the size?
JCZuurmond
force-pushed
the
feat/create-table-from-query
branch
from
6a5c319
to
bdec5d5
Compare
JCZuurmond
force-pushed
the
feat/create-table-from-query
branch
from
8c091cd
to
8e8e886
Compare
|
Probably #154 has preference over this one |
JCZuurmond
force-pushed
the
feat/create-table-from-query
branch
from
892de1a
to
eb5a33a
Compare
JCZuurmond
changed the base branch from
main
to
feat/add-widget-id-to-query-header
JCZuurmond
changed the title
| width, height = self._default_size | ||
| self.position = dataclasses.replace(position, width=position.width or width, height=position.height or height) | ||
|
|
||
| @property |
There was a problem hiding this comment.
Don't use property
There was a problem hiding this comment.
@nfx : And the size on WidgetMetdata? Should that also not be a property? I recall the reasoning was that it is better to avoid in case of inherentence
There was a problem hiding this comment.
For consistency I remove the property there as well
JCZuurmond
force-pushed
the
feat/create-table-from-query
branch
from
999173e
to
ed03aa5
Compare
|
This is what happens with an invalid query
|
|
Dashboard with table still renders after all changes:
|
| widget_metadata = WidgetMetadata(query_file, 1, 1, 1) | ||
| tile = QueryTile(widget_metadata) | ||
|
|
||
| fields = tile._find_fields() # pylint: disable=protected-access |
There was a problem hiding this comment.
@nfx : I prefer to keep this unit test as is, i.e. with protected access, it is logic which lends itself well for such a unit test and it should be tested well
JCZuurmond
merged commit 4677acb
into
feat/dashboard-as-code-for-ucx
Resolves #127
Resolves #137
Stacked on top of #154