[Django] #31369: Deprecate NullBooleanField

[Django]

#31369: Deprecate NullBooleanField
-------------------------------------+-------------------------------------
Reporter: David | Owner: nobody
Smith |
Type: | Status: new
Cleanup/optimization |
Component: Database | Version: 3.0
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
#29227 allowed `BooleanField` to be null=true, a step towards deprecating
`NullBooleanField`. #27917 also mentions an aim to deprecate this field.

[https://github.com/django/django/pull/8467#discussion_r175504425
Comments] on the PR discussed deprecating `NullBooleanField` with the
conclusion being that we should wait a while. Now that we are almost 2
years on, and with Django 4.0 being sometime away I suggest now is a good
time to revisit this decision.

This ticket therefore proposes we deprecate `NullBooleanField`. It would
be good to also tidy up the associated fields / widgets e.g. #23681
discussed deprecation of `NullBooleanSelect`

Note: I was slightly unsure if this is best raised as a ticket or as a
topic on the mailing list.

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

[Django]

#31369: Deprecate NullBooleanField
-------------------------------------+-------------------------------------
Reporter: David Smith | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: needsinfo
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: => needsinfo

Comment:

I've posted to the mailing list to get feedback.
https://groups.google.com/d/topic/django-developers/pSvFXcUUkRs/discussion

I'm inclined to Accept, but let's see what folks say.
(I'll close as needsinfo for the moment.)

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

[Django]

#31369: Deprecate NullBooleanField
-------------------------------------+-------------------------------------
Reporter: David Smith | Owner: nobody
Type: | Status: new
Cleanup/optimization |

Component: Database layer | Version: 3.0
(models, ORM) |
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 Carlton Gibson):

* status: closed => new
* resolution: needsinfo =>
* stage: Unreviewed => Accepted

Comment:

Support on the mailing list, so let's progress. Thanks David.

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

[Django]

#31369: Deprecate NullBooleanField
-------------------------------------+-------------------------------------
Reporter: David Smith | Owner: nobody
Type: | Status: new
Cleanup/optimization |

Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Hasan Ramezani):

* has_patch: 0 => 1

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

[Django]

#31369: Deprecate NullBooleanField
-------------------------------------+-------------------------------------
Reporter: David Smith | Owner: nobody
Type: | Status: new
Cleanup/optimization |

Component: Database layer | Version: 3.0
(models, ORM) |
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 Hasan Ramezani):

* has_patch: 1 => 0

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

[Django]

#31369: Deprecate NullBooleanField
-------------------------------------+-------------------------------------
Reporter: David Smith | Owner: hristiy4n
Type: | Status: assigned

Cleanup/optimization |
Component: Database layer | Version: 3.0
(models, ORM) |
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 hristiy4n):

* owner: nobody => hristiy4n
* status: new => assigned

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

[Django]

#31369: Deprecate NullBooleanField
-------------------------------------+-------------------------------------

Reporter: David Smith | Owner: hristiy4n
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 3.0
(models, ORM) |
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
-------------------------------------+-------------------------------------

Comment (by hristiy4n):

"Deprecate NullBooleanField" is a bit of a broad naming. If I understand
correctly, for now just adding a "NullBooleanField has been deprecated."
message should be enough, and later on removing the field in favor of the
BooleanField.

{{{
system_check_deprecated_details = {
'msg': (
'NullBooleanField has been deprecated. Support '
'for it (except in historical migrations) will be removed '
'in Django 4.0.'
),
'hint': (
'Use BooleanField(null=true) instead.'
),
'id': 'fields.W903',
}
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31369#comment:6>

[Django]

#31369: Deprecate NullBooleanField
-------------------------------------+-------------------------------------
Reporter: David Smith | Owner: Hristiyan
Type: | Ivanov
Cleanup/optimization | Status: assigned

Component: Database layer | Version: 3.0
(models, ORM) |
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
-------------------------------------+-------------------------------------

Comment (by David Smith):

Hi hristiy4n,

Thanks for picking this up.

I had a brief look at the code before raising the issue but thought the
principle was important to agree before lots of detailed work. I agree a
deprecation message is key to this. I also think a bit more thought at
this stage would be beneficial in the long run. I'd suggest that 'just'
issuing a deprecation warning at this stage we may uncover something later
which is tricky to resolve.

Here are my more detailed notes having spent some time looking at it in
more detail.

There are three elements which we should consider deprecating. Is it just
the first one at this stage, the top two (I think this is the intent), or
a bit more of a wider tidy up and go for all three?

- There is
[https://docs.djangoproject.com/en/3.0/ref/models/fields/#nullbooleanfield
nullboolean model field]
- and
[https://docs.djangoproject.com/en/3.0/ref/forms/fields/#nullbooleanfield
nullbooleanfield field]
- and the associated
[https://docs.djangoproject.com/en/3.0/ref/forms/widgets/#nullbooleanselect
widget]. Note this widget is currently used when `null=True`

My main concern about just adding a deprecation notice is that the two
`BooleanField`s work slightly differently.

The `models.fields.BooleanField` acts more like the
`forms.fields.NullBooleanField` than `forms.fields.BooleanField`. Whilst I
think the preference should be to deprecate both `NullBooleanField`s do we
need to change the behaviour of `forms.fields.BooleanField` to mirror
`models.fields.BooleanField` when `null=True`.

I hope my code sample below, explains it more clearly that what I've
written out above!

{{{

>>> from django.forms.fields import NullBooleanField
>>> f = NullBooleanField()
>>> f.clean("True")
True
>>> f.clean("") # returns null
>>> f.clean(False)
False

>>> from django.forms.fields import BooleanField
>>> f = BooleanField()
>>> f.required = False
>>> f.clean(True)
True
>>> f.clean("")
False
>>> f.clean(False)
False

>>> from django.db.models import fields
>>> f = fields.BooleanField(null=True)
>>> f.blank = True
>>> f.clean(True, "test")
True
>>> f.clean("", "test") # returns null
>>> f.clean(False, "test")
False

}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31369#comment:7>

[Django]

#31369: Deprecate NullBooleanField
-------------------------------------+-------------------------------------

Reporter: David Smith | Owner: Hristiyan
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
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
-------------------------------------+-------------------------------------

Comment (by Hristiyan Ivanov):

I had a quick look at the code myself and I think I understand the problem
you are raising. The behavior of the `BooleanField`s is indeed different,
which is due to `forms.BooleanField`'s `to_python()` method as it uses
`bool(value)` if the value is not False.

I do agree that if `models.NullBooleanField` is getting deprecated,
`forms.NullBooleanField` should also be deprecated and that just adding
the deprecation message will introduce work later on, as the
`forms.BooleanField` will need to be changed.

I will have a look at the widgets that the two form fields are using to
see if there is any substantial difference and if not, I guess we can
alter the `widgets.CheckboxInput` and also deprecate
`widgets.NullBooleanSelect`, as well as improve the `to_python()` method
of `forms.BooleanField` and therefor deprecate all three
`NullBooleanField` related items.

--
Ticket URL: <https://code.djangoproject.com/ticket/31369#comment:8>

[Django]

#31369: Deprecate NullBooleanField
-------------------------------------+-------------------------------------

Reporter: David Smith | Owner: Hristiyan
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Timothy Schilling):

* cc: Timothy Schilling (added)

* has_patch: 0 => 1

Comment:

I [https://github.com/django/django/pull/12636 made an attempt] at the
deprecation of both `NullBooleanField`'s. I left `NullBooleanSelect` alone
for the time being as it seems like a much larger change. If that's a show
stopper, I can take a swing at that as well.

--
Ticket URL: <https://code.djangoproject.com/ticket/31369#comment:9>

[Django]

#31369: Deprecate NullBooleanField
-------------------------------------+-------------------------------------

Reporter: David Smith | Owner: Hristiyan
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by felixxm):

[https://github.com/django/django/pull/12788 PR with
models.NullBooleanField deprecation]

--
Ticket URL: <https://code.djangoproject.com/ticket/31369#comment:10>

[Django]

#31369: Deprecate NullBooleanField
-------------------------------------+-------------------------------------

Reporter: David Smith | Owner: Hristiyan
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"a92cc84b4a206d18a5f1a0eaa47f19add40ff99b" a92cc84b]:
{{{
#!CommitTicketReference repository=""
revision="a92cc84b4a206d18a5f1a0eaa47f19add40ff99b"
Refs #31369 -- Deprecated models.NullBooleanField in favor of
BooleanField(null=True).
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31369#comment:11>

[Django]

#31369: Deprecate NullBooleanField
-------------------------------------+-------------------------------------

Reporter: David Smith | Owner: Hristiyan
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by felixxm):

I'm not sure about removing `forms.NullBooleanField` (see
[https://groups.google.com/d/msg/django-
developers/pSvFXcUUkRs/i2XPuai3AwAJ comment]).

--
Ticket URL: <https://code.djangoproject.com/ticket/31369#comment:12>

[Django]

#31369: Deprecate the model NullBooleanField.

-------------------------------------+-------------------------------------
Reporter: David Smith | Owner: Hristiyan
Type: | Ivanov

Cleanup/optimization | Status: closed

Component: Database layer | Version: 3.0
(models, ORM) |

Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* status: assigned => closed
* resolution: => fixed

Comment:

We agree on [https://groups.google.com/forum/#!msg/django-
developers/pSvFXcUUkRs/i2XPuai3AwAJ the mailing list] that we should
deprecate only the model `NullBooleanField`.

--
Ticket URL: <https://code.djangoproject.com/ticket/31369#comment:13>

[Django]

#31369: Deprecate the model NullBooleanField.
-------------------------------------+-------------------------------------
Reporter: David Smith | Owner: Hristiyan
Type: | Ivanov
Cleanup/optimization | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"d992f4e3c29a81c956d3d616f0bc19701431b26e" d992f4e]:
{{{
#!CommitTicketReference repository=""
revision="d992f4e3c29a81c956d3d616f0bc19701431b26e"
Refs #31369 -- Removed models.NullBooleanField per deprecation timeline.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31369#comment:14>

[Django]

#31369: Deprecate the model NullBooleanField.
-------------------------------------+-------------------------------------
Reporter: David Smith | Owner: Hristiyan
Type: | Ivanov
Cleanup/optimization | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by GitHub <noreply@…>):

In [changeset:"3b62d8c83e3e48d2ed61cfa32a61c56d9e030293" 3b62d8c]:
{{{
#!CommitTicketReference repository=""
revision="3b62d8c83e3e48d2ed61cfa32a61c56d9e030293"
Refs #31369 -- Improved hint message in NullBooleanField's deprecation
warning.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31369#comment:15>