Opened 11 years ago

Closed 11 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: dev
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 11 years ago.

Download all attachments as: .zip

Change History (6)

by Tilman Koschnick, 11 years ago

Attachment: datastructures.diff added

comment:1 by Anton Baklanov, 11 years ago

Component: HTTP handlingUtilities
Triage Stage: UnreviewedAccepted

pull request with py3 support and tests

comment:2 by Anton Baklanov, 11 years ago

Cc: antonbaklanov@… added
Version: 1.5master

comment:3 by Baptiste Mispelon, 11 years ago

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 by Baptiste Mispelon, 11 years ago

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 by Claude Paroz <claude@…>, 11 years ago

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