BaseContext (and it's subclasses) lack emulation of dictionary items()

[Keryn Knight <django@…>]

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.

[Baptiste Mispelon]

Since BaseContext is basically a ChainMap [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

[Keryn Knight <django@…>]

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.

[Baptiste Mispelon]

Marking this as Accepted. It seems like a reasonable feature to add.

[Baptiste Mispelon]

For reference, #18105 seems related.

[Aaron Cannon]

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

[Baptiste Mispelon]

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.

[Aaron Cannon]

First stab at a patch.

[Aaron Cannon]

Any feedback on this patch is welcome.

[FunkyBob]

Just noting that currently ChainMap is implemented in python in py3, so it is back-portable to py2 almost verbatim.

[Tim Graham]

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.

[Mariusz Felisiak]

Closing as "wontfix" per ​David's comment.