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

Meta options are not inherited. #85

Closed
applegrew opened this issue Jul 15, 2012 · 17 comments
Closed

Meta options are not inherited. #85

applegrew opened this issue Jul 15, 2012 · 17 comments
Labels

Comments

@applegrew
Copy link

Hi,

I have the following tables:-

    class BaseTable(tables.Table):
        select_column = tables.CheckBoxColumn(verbose_name='', empty_values=('NA',))

        class Meta:
            attrs = {'class' : 'cool_table'}

    class EditableTable(BaseTable):
        edit_column = EditIconColumn(verbose_name=_('Edit'))

        def set_editable(self, flag):
            if flag:
                self.base_columns['edit_column'].visible = True
            else:
                self.base_columns['edit_column'].visible = False   

    class RoomTable(EditableTable):
        number = tables.Column()
        speciality = tables.Column()
        photo = ImageViewColumn(accessor='room_type.photo')

When I render RoomTable the generated <table> does not have the class attribute. I am able to get that attribute only when I move (or copy) the Meta class from BaseTable to RoomTable.

Is this a bug or I am using it incorrectly?

@bradleyayers
Copy link
Collaborator

Thanks for the report this is a bug. It's now on the top of my todo list.

@applegrew
Copy link
Author

Thanks.

@bradleyayers
Copy link
Collaborator

So I've almost finished implementing a solution to this, and then I remembered that for Meta inheritence I was following the Django style, i.e.

class Meta(Parent.Meta):
    ...

So now I'm starting to wonder whether it is actually a good idea to deviate from that. I suppose it's always been inconsitent anyway, in the first subclass of django_tables2.Table you don't need to do class Meta(django_tables2.Tables):, but for any further subclasses you do.

I suppose you could say though that there is no django_tables2.Table.Meta API, and it only starts at the first subclass. What's your feeling on this? Did you know how Django's model's Meta inheritence work and expect it to be different here? Or were you naive to Django's API and so you just tried what felt right?

@bradleyayers
Copy link
Collaborator

I've just re–read your original issue, and your issue seems to be that you're not even trying to change meta options in your subclass, you're just trying to subclass and add columns.

@applegrew
Copy link
Author

Sorry I don't clearly understand what you are saying, but yes what I did was that felt right. Anyway, I am pretty new to Meta stuff so my approach may not be good.

@jrschneider
Copy link

Bradley,

Love the work on this project. I am running into the same issue as applegrew here, and your last comment matches my case perfectly.

### Table Definitions ###
# parent class 
import django_tables2 as tables

# generic parent class to handle table styling
class MyAppTable(tables.Table):
    class Meta:
        attrs = {'class': 'render-table'}


# subclass in a separate app
from myapp_helpers.tables import MyAppTable
from django_tables2.utils import A
import django_tables2 as tables

class ItemTable(MyAppTable):

    address_1 = tables.Column()
    address_2 = tables.Column()
    address_3 = tables.Column()
    city = tables.Column()
    state = tables.Column()
    zipcode = tables.Column()

    # need this to fix a bug with the meta class not passing through to subclasses in django-tables2
    class Meta(MyAppTable.Meta):
        pass


## Views ##

# class based view in separate app
from django_tables2 import SingleTableView

# Parent class to make pagination the same across views
class MyAppListView(SingleTableView):

    # default of 10 rows in every table on every page
    table_pagination = {"per_page": 10 }
    template_name = 'generic_list.html'


# subclass of view in local app
class ItemListView(MyAppListView):
    model = Item
    table_class = ItemTable

If the class Meta definition is not placed in ItemTable, the meta attributes set in the parent class are not sent through. I would like to have the same attrs sent to all of the tables in my apps (that eventually inherit from MyAppTable)

Defining the empty Meta class in ItemTable that inherits from the parent Meta class (MyAppTable) works, but seems to work against the DRY principle, even though it is only two lines and still meets my requirement of only needing to change the CSS class in one spot should I need to. Appreciate the work! Thank you.

@bradleyayers
Copy link
Collaborator

I know where you're coming from, and I tend to agree with you, in-fact I've partially written a patch to implement this, however this approach contradicts the approach Django takes with models. My current stance however is that having things work in a similar way to Django is important for having the library work as expected.

Do you think it's really worth breaking from Django + Python standard behaviour for this?

@jrschneider
Copy link

Having only developed using Django/Python for about a month, I don't think I'm qualified to make an argument for breaking from the norm. I suppose there are a few options though. You could:

a) Document the workaround as detailed above (if it isn't already, I might have missed it). It's really not that terrible.
b) Provide an attribute like 'inherit_meta = False' that can be overridden in subclasses to allow sub-classes to inherit the meta information. The default would be false to keep things working according to Django standards, but would allow more adventurous/lazy folks such as myself to eliminate the need for the repeated Meta class definitions. The only issue I see with that is maintaining an extra chunk of code for a workaround that could potentially introduce bugs (data present when not expected), etc.
c) Make this behavior the new default.

Even as I write this, I can already see scenarios in my head where template variables are checked based on the fact that inheritance doesn't occur, breakage occurs, and I spend hours poking around the code only to find out that it broke because I overrode the behavior manually. It also wouldn't match any of the existing examples.

Frankly, it probably isn't worth the effort. I would rather see an extra note about the meta inheritance 'gotcha' in the documentation than try to code in a workaround to generate atypical behavior.

@bradleyayers
Copy link
Collaborator

In regards to a), you do it like this:

class Parent(tables.Table):
    class Meta:
        foo = 1

class Child(Parent):
    class Meta(Parent.Meta):
        bar = 2

Child.Meta now has foo = 1 and bar = 2

I tend to agree that this can be fixed with documentation.

@jrschneider
Copy link

Thanks. I actually had this up in my example based on applegrew's example.

@bradleyayers
Copy link
Collaborator

Doh, good point.

@SimonGreenhill
Copy link

One annoyance with the work-around is that you can't then override something in the Parent class:

class Parent(tables.Table):
    class Meta:
        attrs = {...}

class Child(Parent):
    class Meta(Parent.Meta):
         attrs = {'summary': 'A child table'}

raises a NameError with name 'attrs' is not defined. This, however, does seem to work, but is ugly:

class Child(Parent):
    class Meta(Parent.Meta):
        pass
     Meta.attrs['summary'] = 'A child table'

@paulnicholsen27
Copy link

Is there a solution to this bug yet? The above workaround doesn't work for me. http://stackoverflow.com/questions/21787938/inherit-and-modify-a-meta-class

@bradleyayers
Copy link
Collaborator

I don't see an obvious algorithm for merging together two arbitrary class Meta. Think about the cases for adding a key to attrs, removing a key from attrs, replacing attrs with a new dict, etc. Maybe I'm missing something but it's not obvious to me how you would do this in a sane way.

The behaviour should be the same as if you were subclasses models.Model classes.

@paulnicholsen27
Copy link

I'm not sure I follow what you're saying. I'm just trying to inherit the meta from the parent class and add a few columns. I've updated the code in the SO question above, which hopefully makes a bit more sense.

@pydolan
Copy link

pydolan commented Feb 17, 2014

@bradleyayers -- if I'm understanding your request correctly, you should be able to follow this StackOverflow post to overwrite Meta

@bradleyayers
Copy link
Collaborator

@pydolan Yes, check http://stackoverflow.com/questions/21787938/inherit-and-modify-a-meta-class for some other approaches too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants