[Django] #33258: inconsistent use of () and [] for attributes in Admin class - Also class variables

[Django]

#33258: inconsistent use of () and [] for attributes in Admin class - Also class
variables
------------------------------------------+------------------------
Reporter: newearthmartin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 3.2
Severity: Normal | Keywords: Admin
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------+------------------------
In Django's Admin class, most "lists" are initialized as tuples `()` while
two of them (`inlines` and `actions`)are initialized as lists `[]`

```
list_display = ('__str__',)
list_display_links = ()
list_filter = ()
...
list_editable = ()
search_fields = ()
...
inlines = []
...
actions = []
```

This is inconsistent.

And also there is a Since these are declared in the class, they are shared
among all instances if this value is not set.

I wanted to add an action to an admin subclass so I did
`MyAdmin.actions.append('some_action')` which added this action to all my
admins who had not set a new value to `actions`. While lists are more
semantically correct than tuples, tuples have the advantage of being
immutable, so they force users to reset the value every time.

So, two solutions:

- declare all these attributes as instance variables in the constructor.
Possibly setting them all as lists
- declare all these attributes as tuples, keeping them where they are

--
Ticket URL: <https://code.djangoproject.com/ticket/33258>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#33258: inconsistent use of () and [] for attributes in Admin class - Also class
variables

--------------------------------+--------------------------------------

Reporter: newearthmartin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 3.2

Severity: Normal | Resolution:

Keywords: Admin | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------+--------------------------------------
Description changed by newearthmartin:

Old description:

> In Django's Admin class, most "lists" are initialized as tuples `()`
> while two of them (`inlines` and `actions`)are initialized as lists `[]`
>
> ```
> list_display = ('__str__',)
> list_display_links = ()
> list_filter = ()
> ...
> list_editable = ()
> search_fields = ()
> ...
> inlines = []
> ...
> actions = []
> ```
>
> This is inconsistent.
>
> And also there is a Since these are declared in the class, they are
> shared among all instances if this value is not set.
>
> I wanted to add an action to an admin subclass so I did
> `MyAdmin.actions.append('some_action')` which added this action to all my
> admins who had not set a new value to `actions`. While lists are more
> semantically correct than tuples, tuples have the advantage of being
> immutable, so they force users to reset the value every time.
>
> So, two solutions:
>
> - declare all these attributes as instance variables in the constructor.
> Possibly setting them all as lists
> - declare all these attributes as tuples, keeping them where they are

New description:

This is inconsistent.

So, two solutions:

--

--
Ticket URL: <https://code.djangoproject.com/ticket/33258#comment:1>

[Django]

#33258: inconsistent use of () and [] for attributes in Admin class - Also class
variables

--------------------------------+--------------------------------------

Reporter: newearthmartin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 3.2

Severity: Normal | Resolution:

Keywords: Admin | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------+--------------------------------------
Description changed by newearthmartin:

Old description:

> In Django's Admin class, most "lists" are initialized as tuples `()`

> while two of them (`inlines` and `actions`)are initialized as lists `[]`
>
> `
> list_display = ('__str__',)
> list_display_links = ()
> list_filter = ()
> ...
> list_editable = ()
> search_fields = ()
> ...
> inlines = []
> ...
> actions = []
> `
>
> This is inconsistent.
>
> And also there is a Since these are declared in the class, they are
> shared among all instances if this value is not set.
>
> I wanted to add an action to an admin subclass so I did
> `MyAdmin.actions.append('some_action')` which added this action to all my
> admins who had not set a new value to `actions`. While lists are more
> semantically correct than tuples, tuples have the advantage of being
> immutable, so they force users to reset the value every time.
>
> So, two solutions:
>
> - declare all these attributes as instance variables in the constructor.
> Possibly setting them all as lists
> - declare all these attributes as tuples, keeping them where they are

New description:

In Django's Admin class, most "lists" are initialized as tuples `()` while
two of them (`inlines` and `actions`)are initialized as lists `[]`

{{{
list_display = ('__str__',)
list_display_links = ()
list_filter = ()
...
list_editable = ()
search_fields = ()
...
inlines = []
...
actions = []
}}}

This is inconsistent.

And also since these are declared in the class, they are shared among all
instances if this value is not set. This brings the problem where you
inadvertedly add a value to a list and this gets added to all Admin
instances where this attribute has not been set.

ie: I wanted to add an action to an admin subclass so I did

`MyAdmin.actions.append('some_action')` which added this action to all my
admins who had not set a new value to `actions`.

While lists are more semantically correct than tuples, tuples have the
advantage of being immutable, so they force users to reset the value every
time.

So, two solutions:

- declare all these attributes as instance variables in the constructor.
Possibly setting them all as lists
- declare all these attributes as tuples, keeping them where they are

--

--
Ticket URL: <https://code.djangoproject.com/ticket/33258#comment:2>

[Django]

#33258: inconsistent use of () and [] for attributes in ModelAdmin class - Also
class variables
--------------------------------+--------------------------------------

Reporter: newearthmartin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 3.2

Severity: Normal | Resolution:

Keywords: Admin | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------+--------------------------------------
Description changed by newearthmartin:

Old description:

> In Django's Admin class, most "lists" are initialized as tuples `()`

> while two of them (`inlines` and `actions`)are initialized as lists `[]`
>
> {{{
> list_display = ('__str__',)
> list_display_links = ()
> list_filter = ()
> ...
> list_editable = ()
> search_fields = ()
> ...
> inlines = []
> ...
> actions = []
> }}}
>
> This is inconsistent.
>

> And also since these are declared in the class, they are shared among all
> instances if this value is not set. This brings the problem where you
> inadvertedly add a value to a list and this gets added to all Admin
> instances where this attribute has not been set.
>

> ie: I wanted to add an action to an admin subclass so I did

> `MyAdmin.actions.append('some_action')` which added this action to all my
> admins who had not set a new value to `actions`.
>
> While lists are more semantically correct than tuples, tuples have the
> advantage of being immutable, so they force users to reset the value
> every time.
>
> So, two solutions:
>
> - declare all these attributes as instance variables in the constructor.
> Possibly setting them all as lists
> - declare all these attributes as tuples, keeping them where they are

New description:

In Django's `ModelAdmin` class, most "lists" are initialized as tuples

`()` while two of them (`inlines` and `actions`)are initialized as lists
`[]`

{{{
list_display = ('__str__',)
list_display_links = ()
list_filter = ()
...
list_editable = ()
search_fields = ()
...
inlines = []
...
actions = []
}}}

This is inconsistent.

And also since these are declared in the class, they are shared among all

instances if this value is not set. This brings the problem where you
inadvertedly add a value to a list and this gets added to all Admin
instances where this attribute has not been set.

ie: I wanted to add an action to an admin subclass so I did

`MyAdmin.actions.append('some_action')` which added this action to all my
admins who had not set a new value to `actions`.

While lists are more semantically correct than tuples, tuples have the
advantage of being immutable, so they force users to reset the value every
time.

So, two solutions:

- declare all these attributes as instance variables in the constructor.
Possibly setting them all as lists
- declare all these attributes as tuples, keeping them where they are

--

--
Ticket URL: <https://code.djangoproject.com/ticket/33258#comment:3>

[Django]

#33258: inconsistent use of () and [] for attributes in ModelAdmin class - Also
class variables
--------------------------------+--------------------------------------

Reporter: newearthmartin | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 3.2

Severity: Normal | Resolution:

Keywords: Admin | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------+--------------------------------------
Changes (by Carlton Gibson):

* cc: Claude Paroz (added)

Comment:

OK, I think this needs to go to the DevelopersMailingList.

There was some movement towards lists (not necessarily in these exact
locations) in #30947 and [https://github.com/django/django/pull/11267 PR
11267].

The idea was the homogenous lists should lists, where other iterables
where the fields were of different kinds should use tuples.

(I defer the question of which applies where in the case raised here.)

> And also since these are declared in the class, they are shared among

all instances if this value is not set.

> ...

> While lists are more semantically correct than tuples, tuples have the
advantage of being immutable, so they force users to reset the value every
time.

This seems more pressing.
[https://github.com/django/django/commit/97d3321e89c8d4434927bdbc308db1ccffa99d3b#commitcomment-33558481
Claude raise this worry in the discussion on the commit for the PR 11267]

> ... I think we should consider the impact on mutability too. Don't we
get a new risk here where people might suddenly change a class-level list
...

I'm not sure what the best way to balance the considerations is, but I
think it's worth asking for input.

--
Ticket URL: <https://code.djangoproject.com/ticket/33258#comment:4>

[Django]

#33258: inconsistent use of () and [] for attributes in ModelAdmin class - Also
class variables
--------------------------------+--------------------------------------
Reporter: newearthmartin | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution: needsinfo

Keywords: Admin | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------+--------------------------------------
Changes (by Carlton Gibson):

* status: new => closed
* resolution: => needsinfo

Comment:

I'll mark this needs info pending a discussion.

--
Ticket URL: <https://code.djangoproject.com/ticket/33258#comment:5>