Opened 16 months ago

Closed 16 months ago

Last modified 16 months ago

#22413 closed New feature (wontfix)

Allow model instance attributes in ModelAdmin.fields

Reported by: moritzs Owned by: moritzs
Component: contrib.admin Version: master
Severity: Normal Keywords: ModelAdmin instance attribute field
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Say your model uses __getattr__ like that:

class MyModel(models.Model):
    value = models.CharField(max_length=20)

    def __getattr__(self, name):
        if name == 'reversed_value':
            return self.value[::-1]
        else:
            raise AttributeError()

Then you can't use the attribute reversed_value in ModelAdmin.fields because the attribute is only defined in a model instance but not in the class itself.
I suggest adding a field model_instance_fields in ModelAdmin that contains the names of all fields that will be only defined in model instances.

Change History (5)

comment:1 Changed 16 months ago by moritzs

  • Has patch set
  • Needs documentation unset
  • Needs tests set
  • Owner changed from nobody to moritzs
  • Patch needs improvement unset
  • Status changed from new to assigned

pull request: https://github.com/django/django/pull/2534
I will add tests later.

comment:2 follow-up: Changed 16 months ago by timshaffer@…

Forgive my ignorance (perhaps), but what's wrong with changing your code to this:

class MyModel(models.Model):
    value = models.CharField(max_length=20)
    
    @property
    def reversed_value(self):
        return self.value[::-1]

That would not require any changes to Django code.

comment:3 in reply to: ↑ 2 Changed 16 months ago by moritzs

Replying to timshaffer@…:

Forgive my ignorance (perhaps), but what's wrong with changing your code to this:

class MyModel(models.Model):
    value = models.CharField(max_length=20)
    
    @property
    def reversed_value(self):
        return self.value[::-1]

That would not require any changes to Django code.

My example may be a bit trivial, here is another that hopefully shows, why it is necessary:

class MyBaseModel(models.Model):
    def __getattr__(self, name):
        if name.endswith('_to_upper'):
            return getattr(self, name[:-len('_to_upper')]).upper()
        elif name.endswith('_to_lower'):
            return getattr(self, name[:-len('_to_lower')]).lower()
        else:
            return AttributeError()

    class Meta:
        abstract = True


class MyModel(MyBaseModel):
    val1 = models.CharField(max_length=20)
    val2 = models.CharField(max_length=20)

Now it works like this:

>>> m = MyModel()
>>> m.val1 = 'Foo'
>>> m.val2 = 'Bar'
>>> m.val1_to_upper
'FOO'
>>> m.val2_to_lower
'bar'

In this particular example you could still add properties for each combination of val1, val2 and to_upper, to_lower, but this won't be possible, if __getattr__ gets more complex.

comment:4 follow-up: Changed 16 months ago by timo

  • Resolution set to wontfix
  • Status changed from assigned to closed

Thanks for the proposal, but I don't feel this is a common enough use-case to justify the additional complexity. I don't think we should encourage writing really complex __getattr__ methods. As noted earlier, you can add proper methods or properties if you need something in fields.

comment:5 in reply to: ↑ 4 Changed 16 months ago by moritzs

Replying to timo:

Thanks for the proposal, but I don't feel this is a common enough use-case to justify the additional complexity. I don't think we should encourage writing really complex __getattr__ methods. As noted earlier, you can add proper methods or properties if you need something in fields.

How does this add complexity to django? It doesn't break or change the behaviour of ModelAdmin.
Besides, I really consider

class MyModel(models.Model):
    # ... fields

    def __getattr__(self, name):
        # ... do_something

more pythonic than

class MyModel(models.Model):
    # ... fields

    @property
    def field1_do_something(self):
        return do_something(self.field1)

    @property
    def field2_do_something(self):
        return do_something(self.field2)

    ...
Note: See TracTickets for help on using tickets.
Back to Top