Added table_factory #532

ZuluPro · 2018-01-17T01:42:50Z

In the same idea of django.forms.modelform_factory, I created a table_factory.
It helps to create quickly Tables without declare a full class.

I plan to use it in django_tables2.views.SingleTableMixin if table_class isn"t specified.
Creating automaticaly a Table like done in django-filters or Django's ModelFormViews.
(I can do it in this PR ;) )

jieter · 2018-01-17T10:59:12Z

Interesting idea! It might also be possible to replace (some of) the render_table template tag code generating the on the fly table.

What will be an interesting detail is deciding on what kwargs we support. Ideally, not having to define every kwarg in the argument list of modelform_factory would be nice, but I see django doesn't have that (which I assume is for a reason).

For reference: django modelform_factory implementation

Please also take some time to write documentation for this (at least a docstring).

ZuluPro · 2018-01-17T19:32:59Z

OK! Let's do this with kwargs.
I add:

kwargs
Usage in View
Documentation

And I poke you

ZuluPro · 2018-01-18T17:51:41Z

@jieter Poke !

jieter

Thanks, I've added some comments to your diff, please share your opinion if you do not agree.

The function also has to be added to https://github.com/jieter/django-tables2/blob/master/docs/pages/api-reference.rst in order to appear in the documentation.

jieter · 2018-01-18T18:04:11Z

django_tables2/__init__.py

@@ -1,5 +1,5 @@
 # coding: utf-8
-from .tables import Table, TableBase


the string 'table_factory' should be added to __all__, the # noqa comment should be obsolete then.

jieter · 2018-01-18T18:13:47Z

django_tables2/tables.py

@@ -620,3 +620,39 @@ class Table(TableBase):
    __doc__ = TableBase.__doc__

 # Table = DeclarativeColumnsMetaclass(str('Table'), (TableBase, ), {})
+
+
+def table_factory(model, table=Table, **kwargs):


I'd say, if **kwargs doesn't allow passing arbitrary argument to the metaclass, it provides not much benefit to not explicitly define the arguments this function expects.

So either allow pass-through of arguments in some way, or be explicit about what arguments the function takes.

jieter · 2018-01-18T18:14:17Z

django_tables2/tables.py

+    Meta = type(str('Meta'), parent, attrs)
+    # Give this new form class a reasonable name.
+    class_name = model.__name__ + str('Table')
+    # Class attributes for the new form class.


seems to be a leftover from copying modelform_factory?

jieter · 2018-01-18T18:16:11Z

django_tables2/views.py

-        )
+        if self.table_class is None:
+            self.table_class = tables.table_factory(self.model)
+        return self.table_class


What if this is not used with ListView? I'd say if it is mixed in a class not defining self.model, it should still raise the exception.

jieter · 2018-01-18T18:17:24Z

tests/test_views.py

@@ -35,6 +35,10 @@ class SimpleView(DispatchHookMixin, tables.SingleTableView):
    model = Region  # required for ListView


+class SimpleNoTableClassView(DispatchHookMixin, tables.SingleTableView):


You don't seem to use the functionality DispatchHookMixin provides, if so, please remove.

ZuluPro · 2018-01-18T20:01:31Z

OK NP ;)

ZuluPro · 2018-01-19T01:48:55Z

Hello @jieter

I'd say, if **kwargs doesn't allow passing arbitrary argument to the metaclass, it provides not much benefit to not explicitly define the arguments this function expects.

Yes, I think to declare Meta's attributes explicitly is the best idea. Maybe in future I'll declare Table's attributes too.

ZuluPro · 2018-01-21T15:44:03Z

@jieter Any news ?

jieter

Good work! I've added another couple of comments.

jieter · 2018-01-22T07:14:41Z

django_tables2/views.py

+                    "{0}.table_class".format(klass)
+                )
+            self.table_class = tables.table_factory(self.model)
+        return self.table_class


Can you undo the nesting here:

if self.table_class: return self.table_class if self.model: return table_factory(self.model) raise ImproperlyConfigured(...)

I think saving to self.table_class is not needed.

Also: the error message has table_class twice, one must be model.

jieter · 2018-01-22T07:16:02Z

docs/pages/generic-mixins.rst

@@ -13,7 +13,8 @@ template.

 The following view parameters are supported:

- ``table_class`` –- the table class to use, e.g. ``SimpleTable``
+- ``table_class`` –- the table class to use, e.g. ``SimpleTable``, if not specfied
+  a default table will be provided.


This sentence needs to mention the default table will be based on self.model and having a self.model defined is required.

jieter · 2018-01-22T07:17:13Z

tests/test_models.py

+        class Meta:
+            fields = ('first_name',)
+
+        tables.table_factory(Person, localize=localize)


assertions missing.

jieter · 2018-01-22T07:19:20Z

tests/test_views.py

+        self.assertEqual(table_class, view.table_class)
+
+    def test_get_tables_class_auto(self):
+        view = SimpleNoTableClassView()


I prefer having the definition of tables/views in the test as much as possible, so if only used in one test, please put SimpleNoTableClassView in this method.

ZuluPro · 2018-01-23T12:02:25Z

Poke @jieter

jieter · 2018-01-23T12:13:01Z

Merged, thanks.

jieter · 2018-01-27T08:41:46Z

released 1.18.0

ZuluPro added 3 commits January 16, 2018 23:59

Added table_factory

5d148f0

Test table_factory

0455a10

Improved .gitignore

6f8892e

ZuluPro force-pushed the factory branch from 37cbd46 to 5f457e8 Compare January 17, 2018 11:21

Added table_factory tests

c3ecde8

ZuluPro force-pushed the factory branch from 5f457e8 to c3ecde8 Compare January 17, 2018 11:42

ZuluPro added 3 commits January 18, 2018 14:37

Added table_factory in View

b48a379

Added tests

9b4dde7

Added table_factory docs

1221c70

ZuluPro force-pushed the factory branch from a16e764 to 1221c70 Compare January 18, 2018 17:39

jieter requested changes Jan 18, 2018

View reviewed changes

ZuluPro force-pushed the factory branch 3 times, most recently from 4c48767 to 53c3139 Compare January 19, 2018 01:09

jieter reviewed Jan 22, 2018

View reviewed changes

ZuluPro force-pushed the factory branch from 53c3139 to 135391f Compare January 23, 2018 02:14

Little fixes

5a8fdfe

ZuluPro force-pushed the factory branch from 135391f to 5a8fdfe Compare January 23, 2018 02:29

jieter merged commit ffb0c86 into jieter:master Jan 23, 2018

This was referenced Jan 27, 2018

Update django-tables2 to 1.18.0 Karaage-Cluster/karaage#384

Closed

Update django-tables2 to 1.18.0 mwolff44/pk-sbc#133

Closed

This was referenced Jan 29, 2018

Scheduled weekly dependency update for week 04 rlmv/doc-trips#109

Closed

Update django-tables2 to 1.19.0 Karaage-Cluster/karaage#388

Closed

Update django-tables2 to 1.19.0 mwolff44/pk-sbc#139

Closed

pyup-bot mentioned this pull request Feb 5, 2018

Scheduled weekly dependency update for week 05 rlmv/doc-trips#110

Closed

pyup-bot mentioned this pull request Feb 12, 2018

Scheduled weekly dependency update for week 06 rlmv/doc-trips#111

Merged

pyup-bot mentioned this pull request Feb 21, 2018

Update django-tables2 to 1.19.0 Karaage-Cluster/karaage#393

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added table_factory #532

Added table_factory #532

ZuluPro commented Jan 17, 2018

jieter commented Jan 17, 2018

ZuluPro commented Jan 17, 2018

ZuluPro commented Jan 18, 2018

jieter left a comment

jieter Jan 18, 2018

jieter Jan 18, 2018

jieter Jan 18, 2018

jieter Jan 18, 2018

jieter Jan 18, 2018

ZuluPro commented Jan 18, 2018

ZuluPro commented Jan 19, 2018

ZuluPro commented Jan 21, 2018

jieter left a comment

jieter Jan 22, 2018

jieter Jan 22, 2018

jieter Jan 22, 2018

jieter Jan 22, 2018

ZuluPro commented Jan 23, 2018

jieter commented Jan 23, 2018

jieter commented Jan 27, 2018

		@@ -1,5 +1,5 @@
		# coding: utf-8
		from .tables import Table, TableBase

		@@ -35,6 +35,10 @@ class SimpleView(DispatchHookMixin, tables.SingleTableView):
		model = Region # required for ListView


		class SimpleNoTableClassView(DispatchHookMixin, tables.SingleTableView):

Added table_factory #532

Added table_factory #532

Conversation

ZuluPro commented Jan 17, 2018

jieter commented Jan 17, 2018

ZuluPro commented Jan 17, 2018

ZuluPro commented Jan 18, 2018

jieter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZuluPro commented Jan 18, 2018

ZuluPro commented Jan 19, 2018

ZuluPro commented Jan 21, 2018

jieter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZuluPro commented Jan 23, 2018

jieter commented Jan 23, 2018

jieter commented Jan 27, 2018