Issue 1528167: Tweak to make string.Templates more customizable (original) (raw)

Created on 2006-07-25 05:15 by whit537, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
mapping-dot-update.patch whit537,2006-08-01 14:07 s/_multimap/mapping.update/ -- better customizability (against trunk/ r51012)
Messages (18)
msg50742 - (view) Author: Chad Whitacre (whit537) Date: 2006-07-25 05:15
Python's $-style templating is wired for optional case-insensitivity under the hood, but doesn't expose this via the API. The attached patch (against trunk/ r50813) adds a new optional argument to turn this behavior on, and includes doc and tests.
msg50743 - (view) Author: Chad Whitacre (whit537) Date: 2006-07-26 19:50
Logged In: YES user_id=340931 Warning: I've discovered that I introduced a bug. New patch forthcoming.
msg50744 - (view) Author: Chad Whitacre (whit537) Date: 2006-07-31 18:13
Logged In: YES user_id=340931 I've replaced the patch with one that polishes off a few bugs and ambiguities, with accompanying tests and doc.
msg50745 - (view) Author: Chad Whitacre (whit537) Date: 2006-07-31 18:35
Logged In: YES user_id=340931 (BTW, new patch is against trunk/ r51008)
msg50746 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2006-07-31 22:32
Logged In: YES user_id=849994 The patch looks very thorough and complete, I will try to look into it after 2.5 is out. Don't let that prevent you reviewing 5 patches, Chad ;-)
msg50747 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2006-07-31 23:12
Logged In: YES user_id=80475 Barry, is this something you want or is it at odds with the notion of "simplified templating"?
msg50748 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2006-08-01 03:56
Logged In: YES user_id=12800 I'm not psyched about the patch. First, I've always thought that case insensitivity ought to be handled by the mapping from which the keys are being extracted and by setting the idpattern. Second, I definitely don't like adding the case_sensitive argument to substitute() and safe_substitute(). Because it lives in the same namespace as kws it makes for icky rules on resolving any conflicts. Is there a reason why the existing mechanisms are insufficient?
msg50749 - (view) Author: Chad Whitacre (whit537) Date: 2006-08-01 13:21
Logged In: YES user_id=340931 Thanks for your responses, all. > Is there a reason why the existing mechanisms are > insufficient? The problem is kws: you can't customize it from the outside like you can the mapping argument. If we replaced _multimap with mapping.update(kws), then we'd have our hook and I think I'd be satisfied. Would you be any more psyched about that patch? :)
msg50750 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2006-08-01 13:24
Logged In: YES user_id=12800 Yes, that would be much more acceptable!
msg50751 - (view) Author: Chad Whitacre (whit537) Date: 2006-08-01 14:07
Logged In: YES user_id=340931 Ok, here it is! I added a test but wasn't sure this warranted a doc change. Cf.: http://docs.python.org/dev/lib/node40.html
msg50752 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2006-08-02 02:12
Logged In: YES user_id=12800 Looks good to me. Remind us when we fork the trunk.
msg50753 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2006-08-03 18:14
Logged In: YES user_id=80475 The replacement of _multimap is a semantic change. Formerly, a dictionary passed in as a positional argument is left unmolested; now, it is mutated to reflect kwds. Also, the _multimap setup is an O(1) step while the update() approach is O(n). Also, the current implementation only requires the positional argument to support __getitem__(); now, it requires a more fully compliant duck (one that provides an .update() method). - mapping = _multimap(kws, args[0]) + mapping = args[0] + mapping.update(kws) Current behavior: >>> t = Template('the speedbrown′) >>d=dict(speed=′quick′) >>t.safesubstitute(d,speed=′slow′)′theslowbrown′ >>d′speed′:′quick′Afterthepatch,itreturns′speed′:′slow′.Likewisethefollowingnowworks,butwouldfailafterthepatch: >>t=Template(′thespeed brown') >>> d = dict(speed='quick') >>> t.safe_substitute(d, speed='slow') 'the slow brown' >>> d {'speed': 'quick'} After the patch, it returns {'speed': 'slow'}. Likewise the following now works, but would fail after the patch: >>> t = Template('the speedbrown)>>d=dict(speed=quick)>>t.safesubstitute(d,speed=slow)theslowbrown>>dspeed:quickAfterthepatch,itreturnsspeed:slow.Likewisethefollowingnowworks,butwouldfailafterthepatch:>>t=Template(thespeed brown') >>> class D: def __getitem__(self, key): return 'quick' >>> t.safe_substitute(D(), speed='slow') 'the slow brown' The whole approach is misguided, and the use case itself is suspect. If some change is warranted, consider a cleaner, more general-purpose solution like having an optional key= argument to the Template constructor for pre-processing keys: >>> t = Template('the $speed brown', key=str.lower)
msg50754 - (view) Author: Chad Whitacre (whit537) Date: 2006-08-03 23:52
Logged In: YES user_id=340931 Raymond, Thanks for the attention to detail. But could you help me understand what it means that "the whole approach is misguided, and the use case itself is suspect[?]" The customization use case? The approach to customization via the mapping argument? In other words, I understood the mapping argument to already be a customization mechanism; otherwise, the methods could use kws only. So wouldn't a 'key' argument be *more* complicated, introducing additional customization semantics instead of working with what's already there? My concern, though, is that by using _multimap instead of update, we limit the customizations one may perform from the outside. Why this limitation? Performance? Ok. So then it's a matter of adjudicating a performance/feature trade-off?
msg110492 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-16 20:51
Anybody interested in this, if not I'll close it.
msg110820 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-19 22:27
Closing as noone has responded.
msg169662 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-09-01 18:59
I don't think this closure was appropriate. The idea was accepted twice and argued against once, so it isn't dead (it's just resting).
msg264956 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-05-06 09:57
I seconded Raymond. Now the approach is argued against twice. And the patch itself is outdated. I suggest to close this issue. If somebody want to implement Raymond's idea, his can reopen this issue or open new issue.
msg264978 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2016-05-06 15:19
Agreed with Serhiy. Raymond's approach is nicer, but I'm still not convinced about the use case. Closing. If someone wants to run with this again, an updated patch would be needed, and the issue could be reopened, but I also suggest following through with Raymond's idea.
History
Date User Action Args
2022-04-11 14:56:19 admin set github: 43716
2016-05-06 15:19:22 barry set status: open -> closedresolution: wont fixmessages: +
2016-05-06 09:57:50 serhiy.storchaka set nosy: + serhiy.storchakamessages: +
2014-02-03 19:09:14 BreamoreBoy set nosy: - BreamoreBoy
2012-09-01 18:59:31 r.david.murray set status: closed -> openassignee: barry -> versions: + Python 3.4, - Python 3.2nosy: + r.david.murraymessages: + resolution: wont fix -> (no value)
2010-07-19 22:27:47 BreamoreBoy set status: pending -> closedresolution: wont fixmessages: +
2010-07-16 20:51:19 BreamoreBoy set status: open -> pendingversions: + Python 3.2, - Python 2.7nosy: + BreamoreBoymessages: +
2009-03-30 04:35:24 ajaksu2 set stage: patch reviewtype: enhancementversions: + Python 2.7, - Python 2.6
2006-07-25 05:15:58 whit537 create