On Fri, Jun 08, 2007 at 11:05:01PM -0000, SmileyChris wrote:
> 
> On Jun 9, 9:32 am, Brian Harring <ferri...@gmail.com> wrote:
> > Realize it's hit trunk already,
> No worries, we can always open another ticket :)

Already watching enough tickets ;)


> > but noticed an annoying potential
> > gotcha- for single var in a for loop, any changes to the context stay
> > put- for multiple vars, the context gets wiped.
> Dang, I knew there must have been some logic in why I was handling the
> context originally - this was the case I had thought of then promptly
> forgotten about.
> 
> You're right. This is a change in behaviour between a single and
> multiple keywords and probably should be addressed.
> This would mean changing the context.update behaviour like mentioned
> in the other ticket in this thread and and making sure that any
> keywords not used this loop get popped out.

Related to that, context.(push|pop) really ought to return the newly 
(add|remov)ed scope- if you have direct access to the underlying scope 
object, you can safely bypass the context stack object and abuse dict 
methods directly (its update, clear, etc specifically).  'Safely' in 
this case means "without having to know the internals of Context".

Two upshots to it, you avoid involving Context.__.*item__ unless you 
specifically need it, more importantly it's a mild api tweak that 
is backwards compatible and removes the need to change the 
Context.update behaviour you're proposing.

For consistancy, if that were changed Context.update ought to return 
the newly added scope, but again, it's backwards compatible- 
if anyone was already relying on Context.(push|pop|update) returning 
None, well, they're special and need to be wedgied for relying on 
basically a void return :)


> > So... consistancy would be wise; which should it be?

Replying to myself since I've had some time to think about it, yet to 
see any code that relies on the parent preserving random mappings in 
the context- thinking wiping the context per iteration might be wise.  

Tags shouldn't really be leaving random cruft in the parent context, 
thus cleansing the owned scope per iteration avoids the potential 
for screwups.

Additionally, it'll simplify ForNode.renders code flow- also will 
scale sanely (context.(push|pop) are popleft's on a list- meaning 
deeper the context, worse the performance).


> > Also, use izip instead of zip dang it :P
> Gosh, how many keyword arguments are you trying to use? ;)
> Does it really matter to use izip if it's going to usually be less
> than 3 iterations?

izip vs zip in this particular case is a rough 20% gain in izips 
favor for < 10 args raw izip/zip speed; honestly it's a background 
gain, it's a good habit to have however for dict bits however (plus it 
was a joke).

Had a nice funny rant typed up related to the general lack of code 
efficiency in template code (ticket 4523 is an exmaple), but instead 
just going to point out that adding new good habits to your 
repertoire isn't a bad thing.  Knowing weird bits like above aren't a 
bad thing- no different then knowing that

if len(sequence):
  pass

is daft- the only time that's sane is if sequence.__nonzero__ isn't 
properly defined (which means it should be properly defined since it 
goes to the trouble of defining sequence.__len__ already- hacking 
around the lack of __nonzero__ via len is fugly).  Better habit that 
avoids the extra func calls and extra object is-

if sequence:
  pass

As said, good habits.  Simpler, more often then not, faster.  Using 
izip instead of zip for dict updates/creation is pretty much always a 
win (even on a single item), thus a similar 'good habit'.

So as I said, "use izip instead of zip dang it :P".  Good habits such 
as that helps to avoid "death by a thousand cuts" syndrome in your 
code.

~harring