Opened 11 years ago
Closed 11 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 , 11 years ago
| Version: | 1.6 → master |
|---|
comment:2 by , 11 years ago
| Type: | Uncategorized → New feature |
|---|
comment:3 by , 11 years ago
| Cc: | added |
|---|---|
| Owner: | changed from to |
| Status: | new → assigned |
comment:4 by , 11 years ago
| Cc: | removed |
|---|
comment:5 by , 11 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 , 11 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 , 11 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 , 11 years ago
| Patch needs improvement: | unset |
|---|
comment:9 by , 11 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 , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
Created a pull request:
https://github.com/django/django/pull/3541