Opened 10 years ago
Closed 10 years ago
#23838 closed Cleanup/optimization (fixed)
LazyObject is missing __iter__
Reported by: | Rik | Owned by: | Rik |
---|---|---|---|
Component: | Utilities | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | 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
The LazyObject
class in utils.functional
(https://github.com/django/django/blob/master/django/utils/functional.py#L235) is missing the __iter__
magic method.
Other dict related magic methods like __getitem__
, __setitem__
and __delitem__
are supported but not __iter__
. I can't think of a reason it isn't there and haven't been able to find discussions about it. The __getitem__
, __setitem__
and __delitem__
were added in this ticket:
https://code.djangoproject.com/ticket/18447
I think they just forgot __iter__
. I'm proposing to add this. If this is done, then tickets like these:
https://code.djangoproject.com/ticket/23783
Can be resolved with SimpleLazyObject
(which implements LazyObject
).
Change History (10)
comment:1 by , 10 years ago
Version: | 1.6 → master |
---|
comment:2 by , 10 years ago
Type: | Uncategorized → New feature |
---|
comment:3 by , 10 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:4 by , 10 years ago
Cc: | removed |
---|
comment:5 by , 10 years ago
Component: | Uncategorized → Utilities |
---|
As I noted on the pull request, the new tests pass even before the fix is applied so either there is no issue here or the test needs to be modified to actually show what issue is fixed.
comment:6 by , 10 years ago
Has patch: | set |
---|---|
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Accepted |
Type: | New feature → Cleanup/optimization |
Hi,
The feature looks reasonable but as noted by Tim, the tests need some improvement.
Thanks.
comment:7 by , 10 years ago
I updated my pull request, the test now fails when __iter__
is not implemented in LazyObject
, so it's a good unit test now.
comment:8 by , 10 years ago
Patch needs improvement: | unset |
---|
comment:9 by , 10 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
New tests check for __iter__
and fail with old implementation; commit message is fine, lgtm.
The old test was already covered by the actual tests for __getitem__
.
comment:10 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Created a pull request:
https://github.com/django/django/pull/3541