Ping: Extend "for" tag to allow unpacking of lists
SmileyChris
quietly for a couple of months as a design decision and it's a
reasonably common question in the IRC channel so I thought I'd ask a
core developer for a decision.
The ticket itself relies on a change to context.update, which is
probably why it has been delayed but my latest crack at that makes
more sense (http://code.djangoproject.com/ticket/3529).
Anyway, just a pingidy ping.
David Larlet
>
> Ticket http://code.djangoproject.com/ticket/3523 has been sitting
> quietly for a couple of months as a design decision and it's a
> reasonably common question in the IRC channel so I thought I'd ask a
> core developer for a decision.
>
I'm not a core dev but as a daily user, this patch is really
interesting/pythonic so I'm definitely +1.
David
Russell Keith-Magee
Saying the patch is Pythonic isn't a good thing in this context. The
template language is _not_ Python, nor will it ever be.
That's not to say it isn't a good idea. I still have to look at the
details on this one, so I'll withhold judgement until then.
Yours,
Russ Magee %-)
Brian Harring
the eyes reading wise, and it has the nice side affect of speeding up
the access- index access is last in the chain for var resolution,
meaning several pricey exceptions/tests occur every access.
Faster resolution, and cleaner to read.
On Tue, Jun 05, 2007 at 02:34:42AM -0000, SmileyChris wrote:
>
> Ticket http://code.djangoproject.com/ticket/3523 has been sitting
Comments on the patch/syntax at least; Offhand, test
'for-tag-unpack03' points out some uglyness in the api-
{% for key value in items %}
{{ key }}:{{ value }}/
{% endfor %}
given ((1,2), (3,4)) yields
'1:2/3;4'
So which is it? Comma delimted, or space? Realize the parser
forces args coming in split on whitespace, but I don't see the point
in supporting two different delimiters for the 'var names' chunk of
the syntax.
Prefer ',' personally (easier to pick out, among other things), but
one or the other; KISS mainly. Aside from that,
{% for pythons,allowed,trailing,comma,is,not,worth,preserving, in items %}
..super happy template crap...
{% endfor %}
Again, imo, isn't wise. Technically, you can shove in
{% for x,,,,,,y in foon %}{% endfor %}
So the parsing there (the 'filter(None, ...)' specifically), is too
loose imo.
Either way, semantics there differ from python; ',' serves as a
marker (basically) to tuple'ize the surrounding bits-
>>> for x in [(1,),(2,)]:print x
(1,)
(2,)
is different from the rather nifty deref abuse of
>>> for x, in [(1,), (2,)]:print x
1
2
Not sure if supporting the trailing comma deref trick is
worthwhile; either way patch up the loose commas handling and it's no
longer valid syntax, so the issue can be ignored till someone
actually pushes for adding the equiv trick into the template syntax.
Use itertools.izip also (you don't need the intermediate list, thus
don't allocate one).
> The ticket itself relies on a change to context.update, which is
> probably why it has been delayed but my latest crack at that makes
> more sense (http://code.djangoproject.com/ticket/3529).
Technically the change in 3529 should support dict.update's "I take
either a sequence of len 2 tuples, or a dict" also- meaning if update
is going to be made closer to dict behavior, the intermediate dict
isn't needed (straight izip in that case).
~harring
SmileyChris
On Jun 5, 11:48 pm, Brian Harring <ferri...@gmail.com> wrote:
> Comments on the patch/syntax at least; Offhand, test
> 'for-tag-unpack03' points out some uglyness in the api-
>
> {% for key value in items %}
> {{ key }}:{{ value }}/
> {% endfor %}
Regarding the whole comma/space issue, the patch simply aimed to be
flexible.
Mostly because I didn't want to be a stickler over comma-space-
variable vs comma-variable.
My assumption would be that comma was the "official" and documented
way to do it (as opposed to just space separation), but I didn't think
that forcing an error was necessary. Maybe I was being to sympathetic
to template designers :)
>
> given ((1,2), (3,4)) yields
>
> '1:2/3;4'
>
> So which is it? Comma delimted, or space? Realize the parser
> forces args coming in split on whitespace, but I don't see the point
> in supporting two different delimiters for the 'var names' chunk of
> the syntax.
>
> Prefer ',' personally (easier to pick out, among other things), but
> one or the other; KISS mainly. Aside from that,
>
> {% for pythons,allowed,trailing,comma,is,not,worth,preserving, in items %}
> ..super happy template crap...
> {% endfor %}
>
> Again, imo, isn't wise. Technically, you can shove in
>
> {% for x,,,,,,y in foon %}{% endfor %}
>
> So the parsing there (the 'filter(None, ...)' specifically), is too
> loose imo.
>
> Either way, semantics there differ from python; ',' serves as a
> marker (basically) to tuple'ize the surrounding bits-
I'd be against introducing this complexity into the tag. Sure it
differs from Python, but as Russ has said, it's not Python!
Perhaps the tag could be tightened to error if there are extraneous
(or missing?) commas, but that is as far as we should go.
> Use itertools.izip also (you don't need the intermediate list, thus
> don't allocate one).
>
> > The ticket itself relies on a change to context.update, which is
> > probably why it has been delayed but my latest crack at that makes
> > more sense (http://code.djangoproject.com/ticket/3529).
>
> Technically the change in 3529 should support dict.update's "I take
> either a sequence of len 2 tuples, or a dict" also- meaning if update
> is going to be made closer to dict behavior, the intermediate dict
> isn't needed (straight izip in that case).
I concur, that is a worthy change
Thanks for your thoughts, Brian!
SmileyChris
small bug):
Russell Keith-Magee
Ok; I've had a chance to look at this now. I've made a couple of minor
modifications, mostly in docs, and to make the parser more forgiving
on whitespace around the separating commas.
Unless there are any objections, I'm happy to check this into trunk.
Yours,
Russ Magee %-)
Jacob Kaplan-Moss
> Unless there are any objections, I'm happy to check this into trunk.
Meh, I'm barely +0, but don't let my apathy stop you :)
Jacob
Adrian Holovaty
Michael Trier
Michael
SmileyChris
> This looks really cool. Thanks SmileyChris.
And Honza for the initial work on this ticket :)
Brian Harring
Realize it's hit trunk already, 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.
Basically, template equiv of
for x in xrange(10):
if x:
print y
y = 'foon'
behaves, while
for x,z in enumerate(xrange(10)):
if x:
print y
y = 'foon'
would cleanse the local scope each iteration, leading to (at best)
string if invalid, worst, pukage.
So... consistancy would be wise; which should it be? Also, use izip
instead of zip dang it :P
~harring
SmileyChris
> 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.
> So... consistancy would be wise; which should it be? 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?
Brian Harring
>
> 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 they 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 it through, 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. Also has the benefit of simplifying ForNode.render
while making it scale better (context.pop is a popleft on a list-
meaning deeper the context stack, performance is linearly worse).
> > 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 (plus it was a
joke, the real improvement is suggested above).
Had a nice funny rant typed up related to the general lack of code
efficiency in template code (ticket 4523 is an example), but instead
just going to point out that adding new good habits to your
repertoire isn't a bad thing. Knowing stuff like above is actually a
*good* 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 code, 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.
Lecture aside, opinions on the proposed scope change for
ForNode.render from above is desired.
~harring