[Django] #30689: Fix documentation for the `fields` argument of MultiValueField
Django
-----------------------------------------+------------------------
Reporter: Adam Sołtysik | Owner: nobody
Type: Uncategorized | Status: new
Component: Documentation | Version: 2.2
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 |
-----------------------------------------+------------------------
In the
[https://docs.djangoproject.com/en/2.2/ref/forms/fields/#multivaluefield
documentation] there is the class header:
{{{class MultiValueField(fields=(), **kwargs)}}}
However, in the
[https://docs.djangoproject.com/en/2.2/_modules/django/forms/fields/#MultiValueField
source code] the `fields` argument has no default value:
{{{def __init__(self, fields, *, require_all_fields=True, **kwargs):}}}
This behaviour seems to have changed in Django 2.0 (some of my forms
started crashing after the upgrade), but there is nothing about it in the
[https://docs.djangoproject.com/en/2.2/releases/2.0/ release notes].
--
Ticket URL: <https://code.djangoproject.com/ticket/30689>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Adam Sołtysik | Owner: nobody
Component: Documentation | Version: master
Severity: Normal | Resolution: invalid
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
* type: Uncategorized => Bug
* version: 2.2 => master
* resolution: => invalid
Comment:
Without actual code it's difficult to know what's broken in your project
but I'd guess the relevant change was #28192. Optional args are now
keyword only. As [https://docs.djangoproject.com/en/2.2/releases/2.0
/#form-fields-no-longer-accept-optional-arguments-as-positional-arguments
per the note in the Backwards incompatible changes section of the 2.0
release notes]:
> To help prevent runtime errors due to incorrect ordering of form field
arguments, optional arguments of built-in form fields are no longer
accepted as positional arguments.
In general, the reference docs do not attempt to match the code signature
of the documented methods exactly. Rather they aim to give a guide to the
interface from the user perspective. In this case, `fields` doesn't have
an actual default, but is a required tuple of fields, as per the
discussion below.
--
Ticket URL: <https://code.djangoproject.com/ticket/30689#comment:1>
Django
Reporter: Adam Sołtysik | Owner: nobody
Component: Documentation | Version: master
Severity: Normal | Resolution: invalid
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Adam Sołtysik):
My problem was that the `fields` argument was not passed to the
constructor of a class inheriting from `MultiValueField`. In Django 1.11
it worked, because empty tuple `()` was the default for this argument,
just like in the class header in the documentation, and since Django 2.0
there is no default value. The exact code change is here:
https://github.com/django/django/commit/8e752d84378c7169ef73a483ffffcba55a08c867.
I think this is a breaking change that should be documented in the release
notes. This:
{{{Form fields no longer accept optional arguments as positional
arguments}}}
is not relevant, since `fields` is in fact accepted as a positional
argument.
--
Ticket URL: <https://code.djangoproject.com/ticket/30689#comment:2>
Django
Reporter: Adam Sołtysik | Owner: nobody
Component: Documentation | Version: master
Severity: Normal | Resolution: invalid
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
8e752d84378c7169ef73a483ffffcba55a08c867 was just correcting a bug: that
`fields` was **not** in fact required, contrary to the documentation.
Sorry that you've hit this, but it doesn't qualify as a breaking change.
(All bugfixes are breaking changes if you're relying on the broken
behaviour...)
--
Ticket URL: <https://code.djangoproject.com/ticket/30689#comment:3>
Django
Reporter: Adam Sołtysik | Owner: nobody
Component: Documentation | Version: master
Severity: Normal | Resolution: invalid
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Adam Sołtysik):
I get it, though I think there's a difference between a bug when something
doesn't work, and a "bug" when something works but it maybe shouldn't.
That's the second time I stumbled upon such a situation -- several days
ago I was refactoring part of the project because of #30333, and the
change was also not documented. What's sad is that it's not my fault, as I
didn't work on the project from the beginning and I didn't write the code
in question, but now I have no way of being sure whether nothing else is
broken after the upgrade. So maybe it's worth considering to add a special
section to release notes just for code-breaking bugfixes ;)
--
Ticket URL: <https://code.djangoproject.com/ticket/30689#comment:4>