Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call to model "__str__" when rendering a table with custom column #511

Closed
jxrossel opened this issue Dec 5, 2017 · 6 comments · Fixed by #514 or rlmv/doc-trips#106
Closed

Call to model "__str__" when rendering a table with custom column #511

jxrossel opened this issue Dec 5, 2017 · 6 comments · Fixed by #514 or rlmv/doc-trips#106

Comments

@jxrossel
Copy link

jxrossel commented Dec 5, 2017

Hi,

When rendering a table associated with a model, the model "str" method is called for each row. In my case, this leads to many unnecessary queries (or the need to use "select_related" for undisplayed data). Can this be avoided ?

@jieter
Copy link
Owner

jieter commented Dec 5, 2017

@jxrossel Can you share a minimal example for the models/table to reproduce this?

@jxrossel jxrossel changed the title Call to model "__str__" when rendering the table Call to model "__str__" when rendering a table with custom column Dec 6, 2017
@jxrossel
Copy link
Author

jxrossel commented Dec 6, 2017

from django.db import models
import django_tables2 as dt

class Device(models.Model):
     
    name = models.CharField( _('name'), max_length=200, unique=True )
    
    def __str__(self):
        print( '__str__ called' )
        return self.name

class DeviceTable(dt.Table):
    class Meta:
        model = Device
        orderable = False
        fields = ['name']
    
    edit = dt.Column( verbose_name=_('Edit'), orderable=False, empty_values=[] )
    render_edit = lambda self: 'lala'
    

The problem occurs only if edit is defined as a custom column with custom rendering. If edit is removed and render_edit is replaced by render_name, it doesn't occur

@jieter
Copy link
Owner

jieter commented Dec 13, 2017

This is a failing test without relying on print statements:
https://github.com/jieter/django-tables2/pull/514/files#diff-327b130c82fa4e83e97f6c1a50a0da1cR444

Stack trace leads to

column.current_value = self.get_cell(column.name)

@jieter
Copy link
Owner

jieter commented Dec 13, 2017

The problem is: __repr__ is called in

raise ValueError('Failed lookup for key [%s] in %r'
, which in turn calls six.text_type(self) in the default implementation.

While I think this should be fixed, I also think your model's __str__ method should be relatively cheap to execute, or if it is, it should cache it's return value.

jieter added a commit that referenced this issue Dec 13, 2017
* Added a test to check the number of times str(model) happens

* Use mock.patch to make the test code a bit cleaner

* Fix #511: do not use __repr__ in error message of Accessor.resolve()
@jxrossel
Copy link
Author

Thanks! Well, in my case, the str is not really expensive, but it requires multiple joins that I didn't need for this particular table. Thanks again for your work on django-tables2!

@jieter
Copy link
Owner

jieter commented Dec 14, 2017

released 1.17.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment