Opened 12 years ago
Closed 4 years ago
#20287 closed New feature (wontfix)
BaseContext (and it's subclasses) lack emulation of dictionary items()
Reported by: | Owned by: | ||
---|---|---|---|
Component: | Template system | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | bmispelon@… | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Given the Context usually behaves like a dictionary (though underlying it is obviously also a stack), I was somewhat surprised that it has no items()
nor iteritems()
methods that would allow it to be iterated as one might expect to, though it does have an __iter__()
method, which could probably be used if this has any merit.
Attachments (1)
Change History (12)
comment:1 by , 12 years ago
Cc: | added |
---|---|
Type: | Uncategorized → New feature |
comment:2 by , 12 years ago
I actually sort of do! Though it is arguably a quirk that has needed fixing for years at the debug_toolbar end too, and I'm tempted to file a bug there additionally, having finally sat down and figured (some of?) the problem out, accidentally, having been inspired to investigate when I was playing around with some code in a pdb
session and tried to iterate over my context, and it threw an exception because the method I guessed would be there, wasn't.
In the template panel for debug_toolbar, it explicitly checks each context_layer
(that is, each dict
inside of the Context.dicts
) if it hasattr('items')
before iterating it, as a seemingly innocuous guard. One way in which the problem arises is once you are carrying around a Context
, where one of the layers/dicts itself is a Context
... this admittedly is probably not an intended feature, but it works anyway because the inner context also emulates dictionary key/value pairs. Debug toolbar can no longer iterate and print that part of the template context, because it doesn't really know about bits of it (treats it as {}
). This is especially apparent in things like django-CMS, where plugins will often render entirely without a context according to the debug_toolbar panel, because of the way contexts are used.
Basically though, any code which depends on, or makes assumptions about each Context dict being an actual-dict may fall over in such cases, but this is the only real world case for supporting more dictionary-like usage, in my experience.
comment:3 by , 12 years ago
Triage Stage: | Unreviewed → Accepted |
---|
Marking this as Accepted
. It seems like a reasonable feature to add.
comment:5 by , 12 years ago
I'm a bit confused on what iterkeys and the like should output. My first guess would be that it should iterate through the union of all the keys of the dictionaries in the stack of dicts, (I.E. no duplicate keys). itervalues should return the same value as calling getitem would for that particular key. However, this does not seem to jive with what itter is yielding, which is each dictionary in the stack of dicts.
Does it make sense to have iter return one thing, while itervalues and iterkeys returns something entirely different? My suggestion would be to add all the various basic map methods to iterate over the keys and values as I described at the beginning of this comment, , and changing iter to match, I.E. it would yield each key and value pair from the stack, where items in later dictionaries in the stack override the earlier ones, and all keys would be returned only once.
I'm just not sure if this would be likely to screw up existing code.
Implementing iterkeys and itervalues and other methods in RenderContext will be quite simple, as they can just proxy to self.dicts[-1].methodname. In fact, the get method that's already in there should probably do that as well, rather than rewriting logic already provided.
Please let me know if any of the above is not clear.
Aaron
comment:6 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:7 by , 12 years ago
Like I mentionned in my first comment, I see BaseContext
as an early implementation of a ChainMap
.
Since ChainMap
is python3-only, we can't really use it (yet) directly but I think it'd be good to copy its implementation so that switching to it will be easier when the time comes.
Also note that #20404 is closely related (implementing .keys()
on context objects.
Edit
Forget what I said about #20404, it's not talking about the same thing at all.
comment:9 by , 11 years ago
Just noting that currently ChainMap is implemented in python in py3, so it is back-portable to py2 almost verbatim.
comment:10 by , 8 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
We're dropping Python 2 in master in a couple weeks, so a patch that makes BaseContext
subclass ChainMap
(if that's appropriate, I haven't looked into the details) can be accepted then.
comment:11 by , 4 years ago
Has patch: | unset |
---|---|
Needs tests: | unset |
Resolution: | → wontfix |
Status: | new → closed |
Triage Stage: | Accepted → Unreviewed |
Closing as "wontfix" per David's comment.
Since
BaseContext
is basically aChainMap
[1], I'd be +1 on the idea, but do you have an example showing how this feature might be useful?[1] http://docs.python.org/3/library/collections.html#chainmap-objects