Code

#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 12 months ago.

Download all attachments as: .zip

Change History (6)

Changed 12 months ago by til

comment:1 Changed 12 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 12 months ago by bak1an

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

comment:3 Changed 12 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 12 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 12 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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.