Opened 9 years ago

Closed 5 years ago

#24858 closed New feature (fixed)

Add support for get_foo_display() with ArrayField

Reported by: Mounir Owned by: Hasan Ramezani
Component: contrib.postgres Version: dev
Severity: Normal Keywords:
Cc: chedi Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Mounir)

I think using ArrayField as a many choices field would be awesome.
Passing choices to ArrayField works fine with MultipleChoiceField on the form.
But calling get_foo_display return a TypeError: unhashable type: 'list', maybe this method need to check if the field is an ArrayField it can return a string representation of the choices separated by comma.

class Example(models.Model):
    CHOICES = (
        (1, 'value1'),
        (2, 'value2'),
        (3, 'value3'),
    )

    multi_choices_array = ArrayField(
        base_field=models.IntegerField(),
        choices=CHOICES,
    )

    # Adding this method will show the values
    def multi_choices_array_display(self):
        result = ''
        choices = dict(self.CHOICES)
        for index, value in enumerate(self.multi_choices_array):
            result += "{0}".format(choices[value])
            if not index == len(self.multi_choices_array) - 1:
                result += ', '
        return result

example = Example.objects.create(multi_choices_array= [1, 2])
example.get_multi_choices_array_display()
# Will raise a Type Error exception
example.multi_choices_array_display()
# Will print 'value1, value2'

Attachments (1)

get_FIELD_display.patch (2.5 KB ) - added by chedi 9 years ago.
get_FIELD_display fix for the ArrayField field

Download all attachments as: .zip

Change History (18)

comment:1 by Tim Graham, 9 years ago

Could you please add a code snippet of the expected behavior?

comment:2 by Mounir, 9 years ago

Description: modified (diff)

comment:3 by Mounir, 9 years ago

Description: modified (diff)

comment:4 by Tim Graham, 9 years ago

Summary: get_foo_display with the ArrayFieldAdd support for get_foo_display() with ArrayField
Triage Stage: UnreviewedAccepted
Type: UncategorizedNew feature
Version: 1.8master

Not sure about feasibility, but the request makes sense. If infeasible, it would be nice if ArrayField didn't create the method which doesn't work.

by chedi, 9 years ago

Attachment: get_FIELD_display.patch added

get_FIELD_display fix for the ArrayField field

comment:5 by chedi, 9 years ago

Cc: chedi added
Easy pickings: set
Has patch: set
Needs tests: set

There is two problems here, the first has to do with the fact that lists are not hashable, so get_multi_choices_array_display
will fail when trying to hash the python object. The second one is more or less the same thing but has to do with the
elements of the choice object, it to has to have the choice element of the tuple hashable.

A quick workaround would be to pass tuples in both instances would be to test if the object is an instance of collections.Hashable and call a field specific function to convert the value if it's not the case.

I hacked a some simple code to show the idea, and it seems to work ok.

comment:6 by Asser Schrøder Femø, 9 years ago

Owner: set to Asser Schrøder Femø
Status: newassigned

in reply to:  description comment:7 by Asser Schrøder Femø, 9 years ago

I think the usage in the description is slightly wrong; if specifying choices for an ArrayField the choices themselves should be lists:


class Example(models.Model):
    CHOICES = (
        ([1, 2], 'value1'),
        ([3, 4], 'value2'),
        ([5], 'value3'),
    )
    multi_choices_array = ArrayField(
        base_field=models.IntegerField(),
        choices=CHOICES,
    )

To have an ArrayField that only accepts certain values on the base field, the usage would probably be:

class Example(models.Model):
    CHOICES = (
        (1, 'value1'),
        (2, 'value2'),
        (3, 'value3'),
    )
    multi_choices_array = ArrayField(
        base_field=models.IntegerField(choices=CHOICES),
    )

Then supposedly you could add a forms.MultipleChoiceField to fill in the array's values?

Neither of the two forms above currently work. I've managed to modify the postgres-specific code to handle the first case (choices being lists on the ArrayField) based on chedi's patch -- see https://github.com/asser/django/tree/ticket_24858 -- and am currently trying to figure out what goes wrong in the validation handling of the second case (choices on the base field).

comment:8 by Tim Graham, 9 years ago

Easy pickings: unset
Needs tests: unset
Patch needs improvement: set

comment:9 by Asser Schrøder Femø, 9 years ago

Open PR at https://github.com/django/django/pull/6381

Fixes the issue with get_FIELD_display.

ArrayField seems to work fine with choices on the base field, unless a forms.MultipleChoiceField is used to access it, but that feels like a separate issue to this one (I think it needs work on the validation in the form fields instead).

comment:10 by Tim Graham, 9 years ago

I left comments for improvement on the pull request. Please uncheck "Patch needs improvement" when you update it so that the ticket goes on the review queue.

comment:11 by Artur Smęt, 8 years ago

Owner: changed from Asser Schrøder Femø to Artur Smęt

comment:12 by Artur Smęt, 8 years ago

Patch needs improvement: unset

comment:13 by Tim Graham, 8 years ago

Patch needs improvement: set

comment:14 by Hasan Ramezani, 5 years ago

Owner: changed from Artur Smęt to Hasan Ramezani
Patch needs improvement: unset
Last edited 5 years ago by Mariusz Felisiak (previous) (diff)

comment:15 by Mariusz Felisiak, 5 years ago

Needs tests: set
Patch needs improvement: set

comment:16 by Hasan Ramezani, 5 years ago

Needs tests: unset
Patch needs improvement: unset

comment:17 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In 153c7956:

Fixed #24858 -- Added support for get_FOO_display() to ArrayField and RangeFields.

_get_FIELD_display() crashed when Field.choices was unhashable.

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