Add shortcuts min_value & max_value to RangeField subclasses

[Bertrand Bordage]

Today, if you want to limit the value of a RangeField subclass instance, you have to use the validators RangeMinValueValidator and RangeMaxValueValidator. This means defining:

from django.contrib.postgres.fields import IntegerRangeField
from django.contrib.postgres.validators import (
    RangeMinValueValidator, RangeMaxValueValidator)
from django.db.models import Model, CharField

class Musician(Model):
    name = CharField(max_length=75)
    years_active = IntegerRange(
        validators=[RangeMinValueValidator(1500),
                    RangeMaxValueValidator(3000)])

It could be way easier and more consistent with other fields to define it like this:

from django.contrib.postgres.fields import IntegerRangeField
from django.db.models import Model, CharField

class Musician(Model):
    name = CharField(max_length=75)
    years_active = IntegerRange(min_value=1500, max_value=3000)

[Bertrand Bordage]

Ideally, I see it implemented without Range[Min|Max]ValueValidator (which could then be removed).

base_field could be initialized only when the RangeField is initialized, while adding [Min|Max]ValueValidator(s) based on min_value and max_value.

The validation could then be done in the same way as ArrayField: we validate each field independently. Validators for the whole range could still be added using validators. This would be something like this:

from django.core.validators import MinValueValidator, MaxValueValidator
from django.db.models import Field

class RangeField(Field):
    base_field_class = None

    def __init__(self, min_value=None, max_value=None):
        validators = []
        if min_value is None:
            validators.append(MinValueValidator(min_value))
        if max_value is None:
            validators.append(MaxValueValidator(max_value))
        self.base_field = self.base_field_class(null=True, blank=True, validators=validators)
        self.base_field.model = self.model
        super(RangeField, self).__init__()

    […]

    def validate(self, value, model_instance):
        super(RangeField, self).validate(value, model_instance)
        low, high = value
        self.base_field.validate(low, model_instance)
        self.base_field.validate(high, model_instance)

    def run_validators(self, value):
        super(RangeField, self).run_validators(value)
        low, high = value
        self.base_field.run_validators(low)
        self.base_field.run_validators(high)

[Bertrand Bordage]

Something is also unclear: if one defines min_value, should we forbid None as the lower bound? And same with max_value and None on the upper bound.

It makes sense since specifying None means "infinite value" as you can read ​in the PostgreSQL docs.

[Tim Graham]

Marc, could you explain the rationale for the current implementation?

[Marc Tamlyn]

The addition of the separate validator classes was done to get better error messages mostly, and I think they are clearer to follow than the proposed code in the comments.

The point of validators being separate is that they can be used in both forms and models (and whatever else, like serialization frameworks etc). So I don't see any good reason to remove them as you'll only need to reimplement that same logic in forms.RangeField.

However, the addition of min_value and max_value could be useful shortcuts, and you have probably identified a breaking condition with None. I probably didn't implement the shortcuts as I though there are likely not so many use cases for this, but that may be an oversight. That said, there are other obvious validators we could have for ranges - min/max restrictions on the bounds (e.g. lower bound can be anything but upper bound must be positive) and a max "spread" (i.e. upper - lower) both spring to mind immediately and would be pretty simple validator classes to add. This could lead to a plethora of keyword arguments added to the field.

Anything added to the model field as a shortcut must be added to the form field as well.

---

Slight off topic ramble: I genuinely don't see anything wrong with the validators= API, I think it's a great (form) API which is chronically underused, mainly because its existence is hidden and documented against with shortcuts like min_value= and clean_foo. This may be for historic reasons (min_value came first?) I'm not sure, but in any case the petulant bit of me wants to reject this proposal on the basis that "validators are awesome, we should make people do more of those". But I digress..

---

[Tim Graham]

I think the validators API is fine too, even if a bit verbose. As Marc said, it was added in Django 1.2 after most fields already had the "shortcut" options. I think adding them here creates more complexity in the long run.

Bertrand, feel free to raise the topic on the DevelopersMailingList if you want to discuss it further. Thanks.