Index.__init__()'s fields parameter shouldn't be mutable

[Flavio Curella]

the __init__ method of the Index class has the following signature (​https://github.com/django/django/blob/master/django/db/models/indexes.py#L15):

    def __init__(self, *, fields=[], name=None, db_tablespace=None):

The fields argument is set to a mutable object, which is usually considered bad practice as it can lead to unexpected results:

• ​http://effbot.org/zone/default-values.htm
• ​http://docs.python-guide.org/en/latest/writing/gotchas/

There are some valid uses of mutable defaults, but I can't see any code taking advantage of the mutability by looking at the code of the Index class.

If there's no good reason for the default to be mutable, it should be changed to:

    def __init__(self, *, fields=None, name=None, db_tablespace=None):
        if fields is None:
            fields = []
        # ...rest of the code

If there's a good reason, I think we should document it in a comment.

[Tim Graham]

An older ​PR suggests accepting a list or tuple and defaulting to fields=(). Thoughts?

[Flavio Curella]

I think it should It should accept a list or a tuple as in ​https://github.com/django/django/pull/9368

Re: defaulting to () vs defaulting to None, I don't really have a strong opinion. I'm used to see the arg=None pattern more often, but that could just be cargo-culting.

[Marc Kirkwood]

Replying to Flavio Curella: I agree with that; looks like the PR you linked to solves the problem better.

[Flavio Curella]

@Tim Graham: I think we can just link ​https://github.com/django/django/pull/9368 to this ticket and use it as it is

[Flavio Curella]

To elaborate: the only reason I think it should support both list and tuples is because we have other places where we do so: for example, Meta.ordering comes to mind.

But I'm with you if you'd rather not persevere with that pattern, and push the user to use tuples whenever possible.

[Atul mishra]

I would like to work on this ticket.

[Tim Graham]

There's already a PR.

[Tim Graham <timograham@…>]

In 10c0fe5:

Fixed #29178 -- Allowed Index.fields to accept a tuple.