[Django] #29656: Range Fields do not support blank values via ModelForm
Django
-----------------------------------------+------------------------
Reporter: James Addison | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
The filing of this issue is based on a discussion in IRC (see
https://botbot.me/freenode/django/2018-08-09/?msg=103129716&page=12 and
prior messages), followed up by creating a test project to reproduce the
issue.
Please do correct me if I am misusing range fields.
----
Saving a modelform with a model's rangefield 2 inputs left empty triggers
an DB integrity error. I think the culprit lies with `empty_values` not
containing `['', '']` as a possible empty value. (see the code around
https://github.com/django/django/blob/1.11.15/django/forms/fields.py#L1026)
With a view like:
{{{
def home(request):
if request.method == 'POST':
form = RangeTestForm(request.POST)
if form.is_valid():
instance = form.save()
else:
form = RangeTestForm(request.POST)
return render(request, 'rangefieldtest/home.html', {'form': form})
}}}
Form like:
{{{
class RangeTestForm(forms.ModelForm):
class Meta:
model = RangeTest
fields = '__all__'
}}}
and Model like:
{{{
from django.contrib.postgres.fields import FloatRangeField
from psycopg2._range import NumericRange
class RangeTest(models.Model):
name = models.CharField(max_length=50, blank=True, default='')
age_range = FloatRangeField(blank=True, default=NumericRange)
}}}
I will attach a sample project demonstrating this (use runserver, load the
home page, click save)
----
psql table definition:
{{{
Table "public.rangefieldtest_rangetest"
Column | Type |
Modifiers
-----------+-----------------------+-----------------------------------------------------------------------
id | integer | not null default
nextval('rangefieldtest_rangetest_id_seq'::regclass)
name | character varying(50) | not null
age_range | numrange | not null
Indexes:
"rangefieldtest_rangetest_pkey" PRIMARY KEY, btree (id)
}}}
data successfully stored on save (when at least one of the rangefield's 2
inputs are filled in):
{{{
id | name | age_range
----+------+-----------
2 | | [3.0,)
(1 row)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29656>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: James Addison | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by James Addison):
* Attachment "rangefield.tgz" added.
sample project reproducing the issue
Django
Reporter: James Addison | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Keywords: | 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 James Addison:
Old description:
New description:
The filing of this issue is based on a discussion in IRC (see
https://botbot.me/freenode/django/2018-08-09/?msg=103129716&page=12 and
prior messages), followed up by creating a test project to reproduce the
issue.
Please do correct me if I am misusing range fields.
----
Saving a modelform with a model's rangefield 2 inputs left empty triggers
an DB integrity error. I think the culprit lies with `empty_values` not
containing `['', '']` as a possible empty value. (see the code around
https://github.com/django/django/blob/1.11.15/django/forms/fields.py#L1026).
'''`self.compress([])` should return the result of `self.range_type(None,
None)` instead of just `None`.'''
With a view like:
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:1>
Django
Reporter: James Addison | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Keywords: | 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 James Addison:
Old description:
> The filing of this issue is based on a discussion in IRC (see
> https://botbot.me/freenode/django/2018-08-09/?msg=103129716&page=12 and
> prior messages), followed up by creating a test project to reproduce the
> issue.
>
> Please do correct me if I am misusing range fields.
>
> ----
>
> Saving a modelform with a model's rangefield 2 inputs left empty triggers
> an DB integrity error. I think the culprit lies with `empty_values` not
> containing `['', '']` as a possible empty value. (see the code around
> https://github.com/django/django/blob/1.11.15/django/forms/fields.py#L1026).
>
> '''`self.compress([])` should return the result of `self.range_type(None,
> None)` instead of just `None`.'''
>
New description:
The filing of this issue is based on a discussion in IRC (see
https://botbot.me/freenode/django/2018-08-09/?msg=103129716&page=12 and
prior messages), followed up by creating a test project to reproduce the
issue.
Please do correct me if I am misusing range fields.
----
Saving a modelform with a model's rangefield 2 inputs left empty triggers
an DB integrity error. I think the culprit lies with `empty_values` not
containing `['', '']` as a possible empty value. (see the code around
https://github.com/django/django/blob/1.11.15/django/forms/fields.py#L1026).
'''`self.compress([])` should return the result of `self.range_type(None,
None)` instead of just `None`.'''
With a view like:
data successfully stored (see id=3 below) on `RangeTest().save()`, which
should also be the result when saving from the modelform:
{{{
test_rangefield=> select * from rangefieldtest_rangetest;
id | name | age_range
----+------+-----------
2 | | [3.0,)
3 | | (,)
(2 rows)
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:2>
Django
Reporter: James Addison | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* version: 1.11 => master
* component: Forms => contrib.postgres
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:3>
Django
Reporter: James Addison | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
It looks like empty range values are currently stored as null and you're
proposing to save them as empty ranges instead. The argument for changing
behavior isn't completely obvious to me and it may be backwards
incompatible.
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:4>
Django
Reporter: James Addison | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by James Addison):
Tim, thanks for responding. I'm (almost?) positive that empty values are
not stored as `null`. They're stored as `(,)` (see my psql `select` output
in the description above).
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:5>
Django
Reporter: James Addison | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by James Addison):
A workaround in the interim; add in the following `.save(...)` override:
{{{
class RangeTest(models.Model):
name = models.CharField(max_length=50, blank=True, default='')
age_range = FloatRangeField(blank=True, default=NumericRange)
def save(self, *args, **kwargs):
if getattr(self, 'age_range') == None:
setattr(self, 'age_range',
self._meta.get_field('age_range').range_type())
super(RangeTest, self).save(*args, **kwargs)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:6>
Django
Reporter: James Addison | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
What I mean is that without any workaround code, Django is trying to save
the empty value as null but you haven't added `null=True` on your
`FloatRangeField`, hence the `IntegrityError`.
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:7>
Django
Reporter: James Addison | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by James Addison):
Very true, I haven't in this example. I did initially start with
`null=True`, but at the time it caused other issues (see below). Just
making sure we're on the same page: shouldn't there be some consistency
between an instance saved via the model's save method (ie
`RangeTest().save()`) and that coming from a modelform's save method?
Otherwise you would always need to check for `None` before accessing
range-specific properties/attributes:
{{{
lval = instance.age_range.lower # this will blow up
if instance.age_range.isempty: # even this will blow up
# do stuff
# these will not blow up, but makes code very verbose!
lval = instance.age_range.lower if instance.age_range is not None else
None
if instance.age_range is not None and instance.age_range.isempty:
# do stuff
}}}
After all, the whole point of not using `null=True` is to ensure a
properly typed value always exists.
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:8>
Django
Reporter: James Addison | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* stage: Accepted => Unreviewed
Comment:
As Tim mentioned I think the behaviour should be consistent with other
non-string based `blank=True` fields.
For example, trying to save an `IntegerField(blank=True)` form field
without providing a default value at save time will also cause an
integrity error because of the non-null constraint. Just like
`IntegerField(blank=True, default=0)` when the user empties the inputs
defaulting to the initial value.
Maybe we could add a note in the `Field.blank` documention to complement
the `Field.blank` mention in `Field.null`'s
[https://docs.djangoproject.com/en/dev/ref/models/fields/#null
documentation].
Something along the line of
> Setting `blank=True` and `null=False` on non-string based fields will
require the addition of logic to handle empty user submitted values.
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:9>
Django
Reporter: James Addison | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by James Addison):
Thanks, I appreciate the reply, but I do think this is sufficiently
different and more impactful to warrant a deeper dive. It's end of day for
me, and my mind is hazy, but I'll give this a go:
- Note that I did not clear any fields, as mentioned in the simpler
example provided (ie. `IntegerField(blank=True, default=0)`).
'''Rangefields with a default value have ''blank'' inputs, so there is
nothing to clear'''. It's not 'user error', really.
- Without a fix for this, using rangefields with modelforms would a)
require either `null=True` all the time to 'work', or b) field validation
to always have either upper or lower bound value set (so, unable to have
unbounded rangefield), or c) the inability to use modelforms with models
containing rangefields
- Lastly, as I mentioned in my comment above, with `null=True` as a 'fix'
it would require using `if instance.age_range is not None` everywhere you
wanted to access PostgreSQL `Range`-based properties like `.lower`, etc.
This would make code incredibly verbose.
I admittedly don't have the same in-depth knowledge of Django as a core
dev, but I think I understand the `IntegerField`'s comparison being made
and if so, I can't exactly agree that it's a valid comparison. Apologies
if this is somewhat rambling and/or blunt - just trying to explain a
user's view.
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:10>
Django
Reporter: James Addison | Owner: nobody
Component: contrib.postgres | Version: master
Severity: Normal | Resolution: duplicate
Keywords: | 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: => duplicate
Comment:
Ultimately this is not anything to do with `RangeField`s per se.
Rather it's just a ''feature'' of how ModelForms interact with ''empty''
input and model field defaults.
Adding an equivalent `IntegerField` to the model, just to show it's not a
`RangeField` thing, all the following is expected behaviour:
{{{
# The model class — note both `number` and `age_range` have `default`s:
class RangeTest(models.Model):
name = models.CharField(max_length=50, blank=True, default='')
number = models.IntegerField(blank=True, default=0)
age_range = FloatRangeField(blank=True, default=NumericRange)
# Then in a shell:
In [1]: from rangefieldtest.models import RangeTest
In [2]: from rangefieldtest.forms import RangeTestForm
In [3]: r = RangeTest(name="No Defaults")
In [4]: r.__dict__
Out[4]:
{'_state': <django.db.models.base.ModelState at 0x108bf32e8>,
'id': None,
'name': 'No Defaults',
'number': 0,
'age_range': NumericRange(None, None, '[)')}
In [7]: r.save()
In [8]: r2 = RangeTest(name="None number", number=None)
In [9]: r2.__dict__
Out[9]:
{'_state': <django.db.models.base.ModelState at 0x108c468d0>,
'id': None,
'name': 'None number',
'number': None,
'age_range': NumericRange(None, None, '[)')}
In [10]: r2.save()
---------------------------------------------------------------------------
IntegrityError Traceback (most recent call
last)
~/ve/tmp-2bf292c1f8d05d6/lib/python3.6/site-
packages/django/db/backends/utils.py in execute(self, sql, params)
63 else:
---> 64 return self.cursor.execute(sql, params)
65
IntegrityError: null value in column "number" violates not-null constraint
DETAIL: Failing row contains (7, None number, (,), null).
In [11]: r3 = RangeTest(name="None Range")
In [12]: r3 = RangeTest(name="None Range", age_range=None)
In [13]: r3.__dict__
Out[13]:
{'_state': <django.db.models.base.ModelState at 0x108d52278>,
'id': None,
'name': 'None Range',
'number': 0,
'age_range': None}
In [14]: r3.save()
---------------------------------------------------------------------------
IntegrityError Traceback (most recent call
last)
~/ve/tmp-2bf292c1f8d05d6/lib/python3.6/site-
packages/django/db/backends/utils.py in execute(self, sql, params)
63 else:
---> 64 return self.cursor.execute(sql, params)
65
IntegrityError: null value in column "age_range" violates not-null
constraint
DETAIL: Failing row contains (8, None Range, null, 0).
In [15]: d = {"name": "Form data"}
In [16]: f = RangeTestForm(data=d)
In [17]: f.is_valid()
Out[17]: True
In [18]: f.cleaned_data
Out[18]: {'name': 'Form data', 'number': None, 'age_range': None}
In [19]: r4 = RangeTest(**f.cleaned_data)
In [20]: r4.__dict__
Out[20]:
{'_state': <django.db.models.base.ModelState at 0x108d7c2b0>,
'id': None,
'name': 'Form data',
'number': None,
'age_range': None}
In [21]: r4.save()
---------------------------------------------------------------------------
IntegrityError Traceback (most recent call
last)
~/ve/tmp-2bf292c1f8d05d6/lib/python3.6/site-
packages/django/db/backends/utils.py in execute(self, sql, params)
63 else:
---> 64 return self.cursor.execute(sql, params)
65
}}}
Here, the `None` value causes the `IntegrityError`. The form gives us the
`None` value. And the model field `default` is **not** used when any value
(even `None`) is provided.
This is what we expect.
What's **wanted** is to use the model field's default value. That's the
same as #27039, so closing as a duplicate.
I think putting the work-around in the model's `save()` is misplacing it.
What we want is the form to give us the right data.
One solution is to override `clean()`:
{{{
# In forms.RangeTestForm...
def clean(self):
cleaned_data = super().clean()
if cleaned_data["number"] is None:
del cleaned_data["number"]
if cleaned_data["age_range"] is None:
del cleaned_data["age_range"]
return cleaned_data
# Then in a shell:
In [1]: from rangefieldtest.models import RangeTest
In [2]: from rangefieldtest.forms import RangeTestForm
In [3]: d = {"name": "Form data"}
In [4]: f = RangeTestForm(data=d)
In [5]: f.is_valid()
Out[5]: True
In [6]: f.cleaned_data
Out[6]: {'name': 'Form data'}
In [7]: r5 = RangeTest(**f.cleaned_data)
In [8]: r5.save()
}}}
An alternative would be to pick out the model field default in a
`clean_<field>` method.
(A custom field/widget combo would also be an option, I guess.)
(It's a matter of taste.)
There are many competing concerns here. The current behaviour has evolved
over a long period. It is a good default. For a specific form, with known
constraints, there's no reason not to override the behaviour where that
suits (as here).
Wanting to use the model default is not an uncommon use-case. Maybe we
could improve the documentation here, but I don't want to leave this open
pending a hypothetical suggestion. (We could review a PR with a new ticket
if/when one comes in.)
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:11>
Django
Reporter: James Addison | Owner: nobody
Component: contrib.postgres | Version: master
Severity: Normal | Resolution: duplicate
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by James Addison):
Carlton,
I know it's discouraged to reopen "core-dev closed" tickets (so I won't!),
but '''I think this ticket should be reopened'''. Let me explain, starting
with an apology: my first attached 'sample' project had a bug where the
the modelform instance was created using `request.POST` as the `data`
input even when loading via `GET`. Originally:
{{{
if request.method == 'POST':
form = RangeTestForm(request.POST)
if form.is_valid():
instance = form.save()
else:
form = RangeTestForm(request.POST)
}}}
... it should have been:
{{{
if request.method == 'POST':
form = RangeTestForm(request.POST)
if form.is_valid():
instance = form.save()
else:
form = RangeTestForm()
}}}
I found this because I naturally followed your `IntegerField` test (good
idea for comparison, by the way). My bug causes any default values that
MIGHT have been used as form defaults to render blank (as `request.POST`
was a empty `dict`) - coming nowhere close to demonstrating the original
problem at all, really. Sorry for the confusion!
That being said, I understand your logic, but it doesn't follow the dev-
user experience with ModelForms. They are typically initially loaded
without data (again, this was my sample code bug), which fills in all
relevant fields with their defaults. ModelForms are request-driven data
entities; using them in a shell-like environment as demonstrated is an
edge case at best - meaning, all fields will be filled in from
`request.POST` data.
- If the user submits the form, field defaults are in `request.POST`, so
the `IntegerField` would normally have a value
- Yes, the user could clear the `IntegerField` and that should trigger
validation/an exception, etc. However, this is the outlying use case;
additional code should be expected
- without user invervention, the `IntegerField`'s default value is
respected on form submit
- In the case of `RangeField`s, an unbounded value in `request.POST` comes
in as `... 'age_range_0': [''], 'age_range_1': [''] ...` (due to the split
widget) which is correct (no bounds are desired as the default), and this
results in `['', '']` for the `RangeField` itself
- without user invervention, the `RangeField`'s default value of `['',
'']` is ''not'' respected on form submit; it makes little sense to treat a
valid default value as an `IntegrityError`; '''how can this not be a
bug?'''
- unbounded ranges are perfectly desirable and are not an outlying case
that should need additional code to allow
- put differently, the current save logic for one possible variation of
a '''valid''' `RangeField` value is to raise an exception
I think I get that from a Django core-dev perspective this is muddying the
waters/codebase (even though I think that the fix is likely as simple as
`empty_values` including `['', '']` for `RangeFields`). But from a Django
dev-user perspective (ok, ok, ''just me at the moment''), what ''should
just work'' doesn't.
I will attach a fixed sample project, which includes the `number` field in
the model.
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:12>
Django
Reporter: James Addison | Owner: nobody
Component: contrib.postgres | Version: master
Severity: Normal | Resolution: duplicate
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by James Addison):
* Attachment "rangefield2.tgz" added.
Updated sample project to fix a bug
Django
Reporter: James Addison | Owner: nobody
Component: contrib.postgres | Version: master
Severity: Normal | Resolution: duplicate
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
I don't see a way forward that wouldn't be backwards incompatible. I
believe that with your proposed change, what are saving as null values
would now be saved as empty ranges. Maybe that's what some people want,
but maybe it's not. You could perhaps argue that the deficiency is the
form widget that doesn't allow distinguishing between null and an empty
range.
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:13>
Django
Reporter: James Addison | Owner: nobody
Component: contrib.postgres | Version: master
Severity: Normal | Resolution: duplicate
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by James Addison):
Tim,
You could perhaps argue that the deficiency is the form widget that
doesn't allow distinguishing between null and an empty range.
I think this is exactly what I'm saying. As I see it, anyone who has
`null=False` (or has omitted it) on a `RangeField` has to have a similar
saving/cleaning hack to mine in place OR they don't use unbounded range
fields. Either way, a fix is appropriate for this scenario, no?
Maybe that's what some people want, but maybe it's not.
I'm not sure what else people in the "`null=False` with a default set"
scenario could possibly want other than the default value to be used if
the form fields were left untouched.
Where it does break... The only scenario where I think allowing `['', '']`
as a possible item in `empty_values` could break is for those who have
`null=True`, as my simplistic fix would replace the current `None` value
for `range([None, None])`. (Just guessing, but I'd think that many of
these would have used `null=True` simply to make range fields 'work' for
them.)
If that is the case, I can think of two possibilities:
1. put fix in place with a deprecation policy:
a. add `['', '']` into `empty_values` for `RangeField`s (addresses the
problem)
b. as a temporary deprecation measure: if `null=True` is used, add a
warning and override the value to be `None` if unbounded
c. in future, remove the deprecation measure as per Django policies
d. maybe advise to create a data migration to move nulls to empty ranges
as necessary
2. put fix in place for `null=False`, but leave `null=True` as is (ie.
always use `None` only if `null=True`)
The latter doesn't feel right to me, but I'm just tossing ideas at the
wall to see if anything sticks. Of course, I may have missed backward
incompatibility considerations - I appreciate any insight!
Lastly... Tim and Carlton, thanks for putting up with my feedback here.
If, after everything mentioned here, you tell me that this won't be fixed
because it is the ''desired'' path forward for Django, then I'll keep my
hack in place and keep quiet. After all, I don't think I can add anything
else at this point.
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:14>
Django
Reporter: James Addison | Owner: nobody
Component: contrib.postgres | Version: master
Severity: Normal | Resolution: duplicate
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Hey James. It’s late for me now so I can’t address each point you make
individually. I will though make time to go over this again just to be
sure.
Right now I’ll just say that I think this issue is a symptom of the more
general problem that HTML forms don’t distinguish properly between empty
string submissions when you mean empty string and empty string submissions
when you mean None. (Or with checkboxes, not submitted vs false.) As such
Django forms have to do their best to behave sensibly in the widest number
of cases. (It’s been many years getting to where it is now: not that it
can’t be improved but it’s pretty battle tested.)
In your case you have a form with a different requirement, but I think
you’ll need to add that logic yourself.
As I said, I will though, have another think about this one.
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:15>
Django
Reporter: James Addison | Owner: nobody
Component: contrib.postgres | Version: master
Severity: Normal | Resolution: duplicate
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
I'm not sure if there's a model field in Django where there's a chance for
two values that could be considered "empty" besides `CharField` and the
range fields. The docs for `Field.null` say, "In most cases, it’s
redundant to have two possible values for “no data;” the Django convention
is to use the empty string, not NULL." I'm not sure if there was much
consideration of null vs. empty range for range field but changing it as
this point merits a discussion on the DevelopersMailingList.
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:16>
Django
Reporter: James Addison | Owner: nobody
Component: contrib.postgres | Version: master
Severity: Normal | Resolution: duplicate
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by James Addison):
Tim, Carlton,
I have little choice but to keep a hack in place (marked with a ''TODO''
note to revisit in future, of course). I don't particularly have a problem
with this, but would like to see consistency and a better user experience
for this in Django.
I'll bring it up on the mailing list, as you recommend. For the most part,
I'll leave it in your hands after that, unless people ask me for feedback.
Thanks again.
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:17>
Django
Reporter: James Addison | Owner: nobody
Component: contrib.postgres | Version: master
Severity: Normal | Resolution: duplicate
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
> `self.compress([])` should return the result of `self.range_type(None,
None)` instead of just `None`.
This **would** be the simplest change. (Just adding `['', '']` to
`empty_values` doesn't quite work, since the comprehension in the lines
you linked goes per-value...)
So in
[https://github.com/django/django/blob/6010da2fbda5eee76b6ec586112561dd26b650e8/django/contrib/postgres/forms/ranges.py#L40-L42
`postgres.forms.ranges.BaseRangeField`]:
{{{
def compress(self, values):
if not values:
return None # Should we use return self.range_type(None,
None) here?
}}}
For now I'd subclass `FloatRangeField` and use
[https://docs.djangoproject.com/en/2.1/topics/forms/modelforms
/#overriding-the-default-fields `field_classes`] to apply it.
--
Ticket URL: <https://code.djangoproject.com/ticket/29656#comment:18>