Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve MultiValueDict/QueryDict dict() return #899

Merged
merged 1 commit into from
Mar 31, 2022

Conversation

intgr
Copy link
Collaborator

@intgr intgr commented Mar 30, 2022

@sobolevn
Copy link
Member

Thanks!

@intgr intgr force-pushed the Fix-MultiValueDict-dict-method branch from 4dd27d7 to 46b3f8d Compare March 31, 2022 08:31
@sobolevn sobolevn merged commit 49d8555 into typeddjango:master Mar 31, 2022
@sobolevn
Copy link
Member

Thank you!

@sterliakov
Copy link
Contributor

It is not completely true:(

from django.utils.datastructures import MultiValueDict

d = MultiValueDict()
d.setlist('foo', [])
print(d.dict())
# {'foo': []}

sterliakov added a commit to sterliakov/django-stubs that referenced this pull request Apr 2, 2022
@intgr
Copy link
Collaborator Author

intgr commented Apr 6, 2022

@sterliakov Thanks, that's a good point I didn't realize.

Most Django users probably only use MultiValueDict via the QueryDict subclass used in request.GET, etc parameters. In this usage, my change is useful, because there's no way to pass a query param value as an empty list.

For my use cases, this practical benefit seems to outweigh the theoretical correctness of the union return.

I'm wondering if there are any reasonable compromises or alternatives that could be implemented?

Or maybe Django developers would be willing to change this behavior.

@intgr intgr deleted the Fix-MultiValueDict-dict-method branch April 6, 2022 11:59
@intgr intgr mentioned this pull request Apr 6, 2022
@intgr
Copy link
Collaborator Author

intgr commented Apr 6, 2022

EDIT: This comment was tangentially related to the dict() method, I opted to move this discussion back to #909 (comment).

@intgr intgr mentioned this pull request Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants