From f5981606e1d0b6bd7dab22195aadbb5720b89911 Mon Sep 17 00:00:00 2001 From: Alex Hall Date: Tue, 20 Jul 2021 22:57:35 +0200 Subject: [PATCH] (core) Make CONTAINS a function for consistency with mkpydocs etc. Summary: Having CONTAINS be a class is a pain, undoing that mistake now Test Plan: none needed Reviewers: dsagal Reviewed By: dsagal Differential Revision: https://phab.getgrist.com/D2929 --- sandbox/grist/autocomplete_context.py | 2 +- sandbox/grist/functions/lookup.py | 11 ++++++++++- sandbox/grist/lookup.py | 6 +++--- sandbox/grist/table.py | 8 ++++---- sandbox/grist/test_summary_choicelist.py | 6 +++--- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/sandbox/grist/autocomplete_context.py b/sandbox/grist/autocomplete_context.py index 997555c7..765fc5b8 100644 --- a/sandbox/grist/autocomplete_context.py +++ b/sandbox/grist/autocomplete_context.py @@ -35,7 +35,7 @@ class AutocompleteContext(object): # TODO It would be nice to include builtin functions too, but getargspec doesn't work there. self._functions = {} for key, value in six.iteritems(self._context): - if value and callable(value) and not isinstance(value, type): + if value and callable(value): argspec = inspect.formatargspec(*inspect.getargspec(value)) self._functions[key] = Completion(key, argspec, is_grist_func(value)) diff --git a/sandbox/grist/functions/lookup.py b/sandbox/grist/functions/lookup.py index d49fbcd2..9f6989d4 100644 --- a/sandbox/grist/functions/lookup.py +++ b/sandbox/grist/functions/lookup.py @@ -151,7 +151,7 @@ def VLOOKUP(table, **field_value_pairs): """ return table.lookupOne(**field_value_pairs) -class CONTAINS(namedtuple("CONTAINS", "value")): +class _Contains(namedtuple("_Contains", "value")): """ Use this marker with `Table.lookupRecords` to find records where a column contains the given value, e.g: @@ -171,4 +171,13 @@ class CONTAINS(namedtuple("CONTAINS", "value")): # While users should apply this marker to values in queries, internally # the marker is moved to the column ID so that the LookupMapColumn knows how to # update its index correctly for that column. + # The _Contains class is used internally, especially with isinstance() + # The CONTAINS function is for users + # Having a function as the interface makes things like docs and autocomplete + # work more consistently pass + +def CONTAINS(value): + return _Contains(value) + +CONTAINS.__doc__ = _Contains.__doc__ diff --git a/sandbox/grist/lookup.py b/sandbox/grist/lookup.py index c7bd64d4..a49b41f1 100644 --- a/sandbox/grist/lookup.py +++ b/sandbox/grist/lookup.py @@ -9,7 +9,7 @@ import records import relation import twowaymap import usertypes -from functions.lookup import CONTAINS +from functions.lookup import _Contains import logger log = logger.Logger(__name__, logger.INFO) @@ -187,7 +187,7 @@ class ContainsLookupMapColumn(BaseLookupMapColumn): # that the columns used to index by are brought up-to-date (in case they are formula columns). group = rec._get_col(extract_column_id(col_id)) - if isinstance(col_id, CONTAINS): + if isinstance(col_id, _Contains): # Check that the cell targeted by CONTAINS() has an appropriate type. # Don't iterate over characters of a string. # group = [] essentially means there are no new keys in this call @@ -300,7 +300,7 @@ class _LookupRelation(relation.Relation): def extract_column_id(c): - if isinstance(c, CONTAINS): + if isinstance(c, _Contains): return c.value else: return c diff --git a/sandbox/grist/table.py b/sandbox/grist/table.py index 0bfcb1b6..95437f77 100644 --- a/sandbox/grist/table.py +++ b/sandbox/grist/table.py @@ -417,12 +417,12 @@ class Table(object): col_ids = [] for col_id in sorted(kwargs): value = kwargs[col_id] - if isinstance(value, lookup.CONTAINS): + if isinstance(value, lookup._Contains): value = value.value # While users should use CONTAINS on lookup values, # the marker is moved to col_id so that the LookupMapColumn knows how to # update its index correctly for that column. - col_id = lookup.CONTAINS(col_id) + col_id = lookup._Contains(col_id) key.append(value) col_ids.append(col_id) col_ids = tuple(col_ids) @@ -455,7 +455,7 @@ class Table(object): c = lookup.extract_column_id(c) if not self.has_column(c): raise KeyError("Table %s has no column %s" % (self.table_id, c)) - if any(isinstance(col_id, lookup.CONTAINS) for col_id in col_ids_tuple): + if any(isinstance(col_id, lookup._Contains) for col_id in col_ids_tuple): column_class = lookup.ContainsLookupMapColumn else: column_class = lookup.SimpleLookupMapColumn @@ -481,7 +481,7 @@ class Table(object): # _summary_source_table._summary_simple determines whether # the column named self._summary_helper_col_id is a single reference # or a reference list. - lookup_value = rec if self._summary_simple else lookup.CONTAINS(rec) + lookup_value = rec if self._summary_simple else lookup._Contains(rec) return self._summary_source_table.lookup_records(**{ self._summary_helper_col_id: lookup_value }) diff --git a/sandbox/grist/test_summary_choicelist.py b/sandbox/grist/test_summary_choicelist.py index c03a5a88..368937e7 100644 --- a/sandbox/grist/test_summary_choicelist.py +++ b/sandbox/grist/test_summary_choicelist.py @@ -142,10 +142,10 @@ class TestSummaryChoiceList(EngineTestCase): {k: type(v) for k, v in self.engine.tables["Source"]._special_cols.items()}, { '#summary#GristSummary_6_Source': column.ReferenceListColumn, - "#lookup#CONTAINS(value='#summary#GristSummary_6_Source')": + "#lookup#_Contains(value='#summary#GristSummary_6_Source')": lookup.ContainsLookupMapColumn, '#summary#GristSummary_6_Source2': column.ReferenceListColumn, - "#lookup#CONTAINS(value='#summary#GristSummary_6_Source2')": + "#lookup#_Contains(value='#summary#GristSummary_6_Source2')": lookup.ContainsLookupMapColumn, # simple summary and lookup @@ -153,7 +153,7 @@ class TestSummaryChoiceList(EngineTestCase): '#lookup##summary#GristSummary_6_Source3': lookup.SimpleLookupMapColumn, '#summary#GristSummary_6_Source4': column.ReferenceListColumn, - "#lookup#CONTAINS(value='#summary#GristSummary_6_Source4')": + "#lookup#_Contains(value='#summary#GristSummary_6_Source4')": lookup.ContainsLookupMapColumn, } )