Opened 6 days ago
Last modified 18 hours ago
#35915 assigned Cleanup/optimization
QueryDict __getitem__ returns an empty list when the value is an empty list
Reported by: | Nguyễn Hồng Quân | Owned by: | jburns6789 |
---|---|---|---|
Component: | HTTP handling | Version: | 5.1 |
Severity: | Normal | Keywords: | querydict |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Update: forum post: https://forum.djangoproject.com/t/change-querydict-getitem-in-case-of-empty-list/36522
Though documentation says that
QueryDict.__getitem__(key)
Returns the value for the given key. If the key has more than one value, it returns the last value.
It returns a list when the raw value is an empty list:
>>> from django.http import QueryDict >>> q = QueryDict('a=1', mutable=True) >>> q['a'] '1' >>> q.setlist('b', []) >>> q['b'] []
which surprises user and it even not mentioned in documentation.
Could we change this behavior? I know that we don't have a perfect solution, but returning None
in this case is less bad than empty list []
, because it is easier to annotate type.
- If returns
None
, we can annotate type asstr | None
. - If returns
[]
, Python doesn't have an exact type for empty list, anddjango-stubs
has to annotate asstr | list[object]
which is quite broader than the actual value (empty list).
Change History (11)
comment:1 by , 6 days ago
Summary: | QueryDict __getitem__ returns list which is surpising → QueryDict __getitem__ returns an empty list when the value is an empty list |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 4 days ago
Owner: | set to |
---|---|
Status: | new → assigned |
follow-up: 5 comment:3 by , 4 days ago
Ok, looking over your suggestion,
I'm trying to figure out what I need to do, here is the doc section for QuerySet
https://docs.djangoproject.com/en/5.1/ref/request-response/#django.http.QueryDict
QueryDict.getitem(key)¶
Returns the value for the given key. If the key has more than one value, it returns the last value. Raises django.utils.datastructures.MultiValueDictKeyError if the key does not exist. (This is a subclass of Python’s standard KeyError, so you can stick to catching KeyError.)
I need to state when a QueryDictkey returns [ ], the said key is associated with a empty list?
comment:4 by , 4 days ago
Owner: | changed from | to
---|
comment:5 by , 4 days ago
Replying to jburns6789:
QueryDict.getitem(key)¶
Returns the value for the given key. If the key has more than one value, it returns the last value. Raises django.utils.datastructures.MultiValueDictKeyError if the key does not exist. (This is a subclass of Python’s standard KeyError, so you can stick to catching KeyError.)
I need to state when a QueryDictkey returns [ ], the said key is associated with a empty list?
Yes
I would be tempted to update this to be very similar to the doc string so maybe
-
docs/ref/request-response.txt
diff --git a/docs/ref/request-response.txt b/docs/ref/request-response.txt index afebd00d8b..abdf75984d 100644
a b a subclass of dictionary. Exceptions are outlined here: 554 554 555 555 .. method:: QueryDict.__getitem__(key) 556 556 557 Returns the value for the given key. If the key has more than one value, 558 it returns the last value. Raises 559 ``django.utils.datastructures.MultiValueDictKeyError`` if the key does not 560 exist. (This is a subclass of Python's standard :exc:`KeyError`, so you can 561 stick to catching ``KeyError``.) 557 Returns the last data value for the given key, or ``[]`` if it's an empty 558 list. Raises ``django.utils.datastructures.MultiValueDictKeyError`` if the 559 key does not exist. (This is a subclass of Python's standard 560 :exc:`KeyError`, so you can stick to catching ``KeyError``.) 562 561 563 562 .. method:: QueryDict.__setitem__(key, value)
comment:6 by , 4 days ago
Owner: | changed from | to
---|
comment:7 by , 3 days ago
Hi, here is my link to the PR, someone else was reviewing, is there anything I need to in the ticket tracker?
comment:9 by , 3 days ago
Has patch: | set |
---|
follow-up: 11 comment:10 by , 2 days ago
Description: | modified (diff) |
---|
comment:11 by , 18 hours ago
Updated requested information https://github.com/django/django/pull/18841
It's been this way a very long time, I'm not sure what the implications of changing it would be (in terms of compatibility). I also think changing it for easier type annotations is not a strong argument here. You could raise this topic on the Django forum and get others opinions if you wish to change it.
That being said, I think a docs adjustment is required (note that the docstring states
Return the last data value for this key, or [] if it's an empty list; raise KeyError if not found.
).Accepting on that basis.