From d79158eadeb499a72aee16fe97e48d8ad6853935 Mon Sep 17 00:00:00 2001 From: Michael Elsdoerfer Date: Tue, 1 Jun 2010 17:28:35 +0200 Subject: [PATCH] No longer allow the Column's 'data' argument to be callable. This was tedious to use anyway, and only made the declared_name/name/data thing more confusing that it already is. Instead, it is now possible to define render_FOO methods within the table. Also, these render methods will receive no the BoundRow as an argument, but rather the actual underlining data row. This makes much more sense, and prevents users from having to fall back to the undocument ``BoundRow.data`` attribute to avoid infinite recursion. --- django_tables/base.py | 30 ++++++++++++++++++++++-------- django_tables/columns.py | 6 ++++++ django_tables/models.py | 30 +++++------------------------- tests/test_memory.py | 37 +++++++++++++++++++++++++++++++------ tests/test_models.py | 27 +++------------------------ 5 files changed, 67 insertions(+), 63 deletions(-) diff --git a/django_tables/base.py b/django_tables/base.py index 686328a..bd8ac9f 100644 --- a/django_tables/base.py +++ b/django_tables/base.py @@ -76,6 +76,7 @@ class DeclarativeColumnsMetaclass(type): attrs['_meta'] = TableOptions(attrs.get('Meta', None)) return type.__new__(cls, name, bases, attrs) + def rmprefix(s): """Normalize a column name by removing a potential sort prefix""" return (s[:1]=='-' and [s[1:]] or [s])[0] @@ -277,6 +278,13 @@ class BoundColumn(StrAndUnicode): # expose some attributes of the column more directly self.visible = column.visible + @property + def accessor(self): + """The key to use when accessing this column's values in the + source data. + """ + return self.column.data if self.column.data else self.declared_name + def _get_sortable(self): if self.column.sortable is not None: return self.column.sortable @@ -343,9 +351,19 @@ class BoundRow(object): """Returns this row's value for a column. All other access methods, e.g. __iter__, lead ultimately to this.""" - # We are supposed to return ``name``, but the column might be - # named differently in the source data. - result = self.data[self.table._cols_to_fields([name])[0]] + column = self.table.columns[name] + + render_func = getattr(self.table, 'render_%s' % name, False) + if render_func: + return render_func(self.data) + else: + return self._default_render(column) + + def _default_render(self, column): + """Returns a cell's content. This is used unless the user + provides a custom ``render_FOO`` method. + """ + result = self.data[column.accessor] # if the field we are pointing to is a callable, remove it if callable(result): @@ -523,11 +541,7 @@ class BaseTable(object): prefix = '' # find the field name column = self.columns[name] - if column.column.data and not callable(column.column.data): - name_in_source = column.column.data - else: - name_in_source = column.declared_name - result.append(prefix + name_in_source) + result.append(prefix + column.accessor) return result def _validate_column_name(self, name, purpose): diff --git a/django_tables/columns.py b/django_tables/columns.py index 5c329cb..5715029 100644 --- a/django_tables/columns.py +++ b/django_tables/columns.py @@ -53,6 +53,11 @@ class Column(object): self.name = name self.default = default self.data = data + if callable(self.data): + raise DeprecationWarning(('The Column "data" argument may no '+ + 'longer be a callable. Add a '+ + '``render_%s`` method to your '+ + 'table instead.') % (name or 'FOO')) self.visible = visible self.inaccessible = inaccessible self.sortable = sortable @@ -72,6 +77,7 @@ class Column(object): direction = property(lambda s: s._direction, _set_direction) + class TextColumn(Column): pass diff --git a/django_tables/models.py b/django_tables/models.py index eeb4ba7..7c7e87f 100644 --- a/django_tables/models.py +++ b/django_tables/models.py @@ -47,33 +47,13 @@ class BoundModelRow(BoundRow): with the instance converted to a dict instead. However, this way allows us to support non-field attributes and methods on the model as well. """ - def __getitem__(self, name): - """Overridden. Return this row's data for a certain column, with - custom handling for model tables. - """ - - # find the column for the requested field, for reference - boundcol = self.table._columns[name] - - # If the column has a name override (we know then that is was also - # used for access, e.g. if the condition is true, then - # ``boundcol.column.name == name``), we need to make sure we use the - # declaration name to access the model field. - if boundcol.column.data: - if callable(boundcol.column.data): - result = boundcol.column.data(self) - if not result: - if boundcol.column.default is not None: - return boundcol.get_default(self) - return result - else: - name = boundcol.column.data - else: - name = boundcol.declared_name - + def _default_render(self, boundcol): + """In the case of a model table, the accessor may use ``__`` to + span instances. We need to resolve this. + """ # try to resolve relationships spanning attributes - bits = name.split('__') + bits = boundcol.accessor.split('__') current = self.data for bit in bits: # note the difference between the attribute being None and not diff --git a/tests/test_memory.py b/tests/test_memory.py index 5c9cfdd..cfb2bfe 100644 --- a/tests/test_memory.py +++ b/tests/test_memory.py @@ -69,6 +69,34 @@ def test_basic(): finally: tables.options.IGNORE_INVALID_OPTIONS = True + +class TestRender: + """Test use of the render_* methods. + """ + + def test(self): + class TestTable(tables.MemoryTable): + private_name = tables.Column(name='public_name') + def render_public_name(self, data): + # We are given the actual data dict and have direct access + # to additional values for which no field is defined. + return "%s:%s" % (data['private_name'], data['additional']) + + table = TestTable([{'private_name': 'FOO', 'additional': 'BAR'}]) + assert table.rows[0]['public_name'] == 'FOO:BAR' + + def test_not_sorted(self): + """The render methods are not considered when sorting. + """ + class TestTable(tables.MemoryTable): + foo = tables.Column() + def render_foo(self, data): + return -data['foo'] # try to cause a reverse sort + table = TestTable([{'foo': 1}, {'foo': 2}], order_by='asc') + # Result is properly sorted, and the render function has never been called + assert [r['foo'] for r in table.rows] == [-1, -2] + + def test_caches(): """Ensure the various caches are effective. """ @@ -117,6 +145,7 @@ def test_meta_sortable(): global_table._meta.sortable = default_sortable assert [c.sortable for c in global_table.columns] == list(results) + def test_sort(): class BookTable(tables.MemoryTable): id = tables.Column(direction='desc') @@ -204,8 +233,9 @@ def test_sort(): assert 'name' in books.order_by assert not 'language' in books.order_by + def test_callable(): - """Data fields, ``default`` and ``data`` options can be callables. + """Data fields and the ``default`` option can be callables. """ class MathTable(tables.MemoryTable): @@ -213,7 +243,6 @@ def test_callable(): rhs = tables.Column() op = tables.Column(default='+') sum = tables.Column(default=lambda d: calc(d['op'], d['lhs'], d['rhs'])) - sqrt = tables.Column(data=lambda d: int(sqrt(d['sum']))) math = MathTable([ {'lhs': 1, 'rhs': lambda x: x['lhs']*3}, # 1+3 @@ -236,10 +265,6 @@ def test_callable(): math.order_by = ('sum',) assert [row['sum'] for row in math] == [1,3,4] - # data function is called while sorting - math.order_by = ('sqrt',) - assert [row['sqrt'] for row in math] == [1,1,2] - # TODO: all the column stuff might warrant it's own test file def test_columns(): diff --git a/tests/test_models.py b/tests/test_models.py index 1c53b58..cfb5a40 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -8,6 +8,7 @@ from django.conf import settings from django.core.paginator import * import django_tables as tables + def setup_module(module): settings.configure(**{ 'DATABASE_ENGINE': 'sqlite3', @@ -50,6 +51,7 @@ def setup_module(module): Country(name="France", tld="fr", population=64, system="republic").save() Country(name="Netherlands", tld="nl", population=16, system="monarchy", capital=amsterdam).save() + def test_declaration(): """Test declaration, declared columns and default model field columns. """ @@ -90,6 +92,7 @@ def test_declaration(): assert not 'population' in CityTable.base_columns # not in Meta:columns assert 'capital' in CityTable.base_columns # in exclude, but only works on model fields (is that the right behaviour?) + def test_basic(): """Some tests here are copied from ``test_basic.py`` but need to be rerun with a ModelTable, as the implementation is different.""" @@ -268,30 +271,6 @@ def test_relationships(): countries.order_by = 'invalid' assert countries.order_by == () -def test_column_data(): - """Further test the ``data`` column property in a ModelTable scenario. - Other tests already touched on this, for example ``test_relationships``. - """ - - class CountryTable(tables.ModelTable): - name = tables.Column(data=lambda d: "hidden") - tld = tables.Column(data='example_domain', name="domain") - default_and_data = tables.Column(data=lambda d: None, default=4) - class Meta: - model = Country - countries = CountryTable(Country) - - # callable data works, even with a default set - assert [row['default_and_data'] for row in countries] == [4,4,4,4] - - # neato trick: a callable data= column is sortable, if otherwise refers - # to correct model column; can be used to rewrite what is displayed - countries.order_by = 'name' - assert countries.order_by == ('name',) - # [bug 282964] this trick also works if the callable is an attribute - # and we refer to it per string, rather than giving a function object - countries.order_by = 'domain' - assert countries.order_by == ('domain',) def test_pagination(): """Pretty much the same as static table pagination, but make sure we -- 2.26.2