From 8ec823e7eed0a61140d11ac23e7660c6eedb5d3e Mon Sep 17 00:00:00 2001 From: Alex Hall Date: Mon, 4 Apr 2022 21:09:48 +0200 Subject: [PATCH] (core) Return RecordSet instead of list from property access when possible, to allow further property access Summary: While `$ref.other_ref` returns a reference (Record) allowing chaining more properties like `$ref.other_ref.foo`, reflists (RecordSet) did not allow this, e.g. `$reflist.other_ref` returned a plain list of records, preventing chaining more dot notation. Discussed here: https://grist.slack.com/archives/CDHABLZJT/p1648845745765839 Test Plan: Added a Python unit test. Formulas like `$reflist.other_ref` were already very common though, and getting the functionality code slightly wrong leads to a flood of test failures. Reviewers: jarek Reviewed By: jarek Subscribers: jarek Differential Revision: https://phab.getgrist.com/D3354 --- sandbox/grist/table.py | 28 ++++++++++++++++++++++++---- sandbox/grist/test_recordlist.py | 17 +++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/sandbox/grist/table.py b/sandbox/grist/table.py index 0bec5eb1..6e64b4cc 100644 --- a/sandbox/grist/table.py +++ b/sandbox/grist/table.py @@ -598,8 +598,8 @@ class Table(object): # Called when record.foo is accessed def _get_col_value(self, col_id, row_id, relation): - [value] = self._get_col_subset(col_id, [row_id], relation) - return value + [value] = self._get_col_subset_raw(col_id, [row_id], relation) + return records.adjust_record(relation, value) def _attribute_error(self, col_id, relation): self._engine._use_node(self._new_columns_node, relation) @@ -607,8 +607,28 @@ class Table(object): # Called when record_set.foo is accessed def _get_col_subset(self, col_id, row_ids, relation): + values = self._get_col_subset_raw(col_id, row_ids, relation) + + # When all the values are the same type of Record (i.e. all references to the same table) + # combine them into a single RecordSet for that table instead of a list + # so that more attribute accesses can be chained, + # e.g. record_set.foo.bar where `foo` is a Reference column. + value_types = list(set(map(type, values))) + if len(value_types) == 1 and issubclass(value_types[0], records.Record): + return records.RecordSet( + values[0]._table, + # This is different from row_ids: these are the row IDs referenced by these Records, + # whereas row_ids are where the values were being stored. + [val._row_id for val in values], + relation.compose(values[0]._source_relation), + ) + else: + return [records.adjust_record(relation, value) for value in values] + + # Internal helper to optimise _get_col_value + # so that it doesn't make a singleton RecordSet just to immediately unpack it + def _get_col_subset_raw(self, col_id, row_ids, relation): col = self.all_columns[col_id] # creates a dependency and brings formula columns up-to-date. self._engine._use_node(col.node, relation, row_ids) - # TODO: when column is a reference, support property access in return value - return [records.adjust_record(relation, col.get_cell_value(row_id)) for row_id in row_ids] + return [col.get_cell_value(row_id) for row_id in row_ids] diff --git a/sandbox/grist/test_recordlist.py b/sandbox/grist/test_recordlist.py index b11250b6..3dd11e64 100644 --- a/sandbox/grist/test_recordlist.py +++ b/sandbox/grist/test_recordlist.py @@ -1,5 +1,7 @@ import testutil import test_engine +from objtypes import RecordSetStub + class TestRecordList(test_engine.EngineTestCase): col = testutil.col_schema_row @@ -83,3 +85,18 @@ class TestRecordList(test_engine.EngineTestCase): [1, "Mammals", [1, 3], [1, 3], True], [2, "Reptilia", [2, 4], [2, 4], True], ]) + + def test_attribute_chain(self): + self.load_sample(self.sample) + self.add_column('Class', 'Names', type='Any', isFormula=True, + formula="$Creatures.Class.Name") + self.add_column('Class', 'Creatures2', type='Any', isFormula=True, + formula="$Creatures.Class.Creatures") + + mammals = RecordSetStub("Creatures", [1, 3]) + reptiles = RecordSetStub("Creatures", [2, 4]) + self.assertTableData("Class", data=[ + ["id", "Name", "Creatures", "Names", "Creatures2"], + [1, "Mammals", [1, 3], ["Mammals", "Mammals"], [mammals, mammals]], + [2, "Reptilia", [2, 4], ["Reptilia", "Reptilia"], [reptiles, reptiles]], + ])