Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#27154 closed Cleanup/optimization (fixed)

Allow comparing CallableFalse/CallableTrue with bitwise or

Reported by: Tom Christie Owned by: Olexander Yermakov
Component: contrib.auth Version: 1.10
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Original raised as a ticket in REST framework: https://github.com/tomchristie/django-rest-framework/issues/4439

Something that may confuse users calling .is_authenticated...

There's a slight difference in behavior between how the CallableFalse/CallableTrue instances behave compare to how False/True behave.

The Python boolean type happens to support the bitwise OR operator, and its behavior in this case is exactly equivalent to the or operator. Eg. False | True returns True

The backwards compat CallableFalse/CallableTrue instances do not support this usage, and will fail with eg. TypeError: unsupported operand type(s) for |: 'instance' and 'bool'

Their behavior is reasonable enough, and the user should really be using the boolean or operator, but we might introduce slightly less friction (ie don't raise a slightly opaque error message to the user) if we simply support and allow the bitwise operator in the same way as the boolean equivalents.

I'm probably a +0 on resolving this, as think it'd save a few folks a bit of head banging. Certainly wouldn't dispute a wontfix categorization.

Tip for easy pickings folks: Should be resolvable by including an __or__ method on the CallableFalse/CallableTrue classes.

Change History (7)

comment:1 by Baptiste Mispelon, 8 years ago

Triage Stage: UnreviewedAccepted

The change seems simple enough and I don't see any drawbacks to adding it.

If we can make our users' lives easier, I'm +1 on it.

If anyone is interested in tackling this, the CallableFalse/CallableTrue classes are defined in django/utils/deprecation.py.

Once the change is made, I think we should also discuss backporting it.

Thanks.

comment:2 by Tim Graham, 8 years ago

Severity: NormalRelease blocker

The implementation can follow commit 54afa960d1ee8c63635225a0f0a2489971b5aab5.

comment:3 by Olexander Yermakov, 8 years ago

Owner: changed from nobody to Olexander Yermakov
Status: newassigned

comment:4 by Olexander Yermakov, 8 years ago

Here is the PR. Thanks.

comment:5 by Tim Graham, 8 years ago

Has patch: set
Summary: Minor difference in behavior between CallableFalse/CallableTrue and False/True.Allow comparing CallableFalse/CallableTrue with bitwise or
Triage Stage: AcceptedReady for checkin

comment:6 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In b7fb6081:

Fixed #27154 -- Allowed comparing CallableBool with bitwise or.

Thanks Tim for the review.

comment:7 by Tim Graham <timograham@…>, 8 years ago

In 7c57f5c:

[1.10.x] Fixed #27154 -- Allowed comparing CallableBool with bitwise or.

Thanks Tim for the review.

Backport of b7fb608142a0be568bc5dce952de5e6aefc2488c from master

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