Opened 4 years ago

Closed 4 years ago

#20276 closed Cleanup/optimization (fixed)

bool(MergeDict()) should evaluate to False if component dicts are empty

Reported by: Tilman Koschnick Owned by: nobody
Component: Utilities Version: master
Severity: Normal Keywords:
Cc: antonbaklanov@…, bmispelon@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently, MergeDict instances are considered True, no matter what the contents of the component dicts are. So you could have 'request.GET or request.POST' evaluate to False and 'request.REQUEST' to True on the same request, which is counter-intuitive.

Adding a __nonzero__ method would fix this.

Attachments (1)

datastructures.diff (349 bytes) - added by Tilman Koschnick 4 years ago.

Download all attachments as: .zip

Change History (6)

Changed 4 years ago by Tilman Koschnick

Attachment: datastructures.diff added

comment:1 Changed 4 years ago by Anton Baklanov

Component: HTTP handlingUtilities
Triage Stage: UnreviewedAccepted

pull request with py3 support and tests

comment:2 Changed 4 years ago by Anton Baklanov

Cc: antonbaklanov@… added
Version: 1.5master

comment:3 Changed 4 years ago by Baptiste Mispelon

Cc: bmispelon@… added

The patch looks good (I added a few minor comments to the PR on github).

Before marking this as RFC, I was wondering if there are potential backwards-compatibility issues? Are there places where MergeDict is used that this patch could impact?

Thanks.

comment:4 Changed 4 years ago by Baptiste Mispelon

Triage Stage: AcceptedReady for checkin

Grepping through django's code, it seems the MergeDict is only used to build request.REQUEST.

I don't see how this patch could break existing code, so I'm marking it as RFC.

Thanks.

comment:5 Changed 4 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: newclosed

In 59d127e45f5ed31e346162a0ac312f672c096821:

Fixed #20276 -- Implemented bool for MergeDict

MergeDict evaluates now to False if all contained dicts are empty.
Thanks til for the report and the initial patch.

Note: See TracTickets for help on using tickets.
Back to Top