#20276 closed Cleanup/optimization (fixed)

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

Reported by: til 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 til 23 months ago.

Download all attachments as: .zip

Change History (6)

Changed 23 months ago by til

comment:1 Changed 23 months ago by bak1an

  • Component changed from HTTP handling to Utilities
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

pull request with py3 support and tests

comment:2 Changed 23 months ago by bak1an

  • Cc antonbaklanov@… added
  • Version changed from 1.5 to master

comment:3 Changed 23 months ago by bmispelon

  • 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 23 months ago by bmispelon

  • Triage Stage changed from Accepted to Ready 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 23 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from new to closed

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