Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default values for string.Template #57382

Closed
nitupho mannequin opened this issue Oct 13, 2011 · 14 comments
Closed

Default values for string.Template #57382

nitupho mannequin opened this issue Oct 13, 2011 · 14 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@nitupho
Copy link
Mannequin

nitupho mannequin commented Oct 13, 2011

BPO 13173
Nosy @warsaw, @birkenfeld, @rhettinger, @merwok, @serhiy-storchaka
Files
  • string_template_default_values.tar: string module, string documentation, and diff file
  • string_template_default_values.patch: diff
  • string_template_default_values2.patch: diff (v.2)
  • string_template_default_values2-light.patch: diff (v.2 "light")
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/warsaw'
    closed_at = <Date 2016-05-06.10:02:23.773>
    created_at = <Date 2011-10-13.21:16:04.899>
    labels = ['type-feature', 'library']
    title = 'Default values for string.Template'
    updated_at = <Date 2016-05-06.10:02:23.772>
    user = 'https://bugs.python.org/nitupho'

    bugs.python.org fields:

    activity = <Date 2016-05-06.10:02:23.772>
    actor = 'serhiy.storchaka'
    assignee = 'barry'
    closed = True
    closed_date = <Date 2016-05-06.10:02:23.773>
    closer = 'serhiy.storchaka'
    components = ['Library (Lib)']
    creation = <Date 2011-10-13.21:16:04.899>
    creator = 'nitupho'
    dependencies = []
    files = ['23398', '23408', '23454', '23455']
    hgrepos = []
    issue_num = 13173
    keywords = ['patch']
    message_count = 14.0
    messages = ['145485', '145531', '145550', '145698', '145699', '145873', '145874', '145876', '145915', '146620', '147252', '163853', '163919', '202568']
    nosy_count = 6.0
    nosy_names = ['barry', 'georg.brandl', 'rhettinger', 'eric.araujo', 'nitupho', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'wont fix'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue13173'
    versions = ['Python 3.4']

    @nitupho
    Copy link
    Mannequin Author

    nitupho mannequin commented Oct 13, 2011

    This patch allows you to define default values for a string.Template, which is useful when you need to use a lot some values, but sometimes other values.

    for example:
    >>> from string import Template
    >>> s = Template("${user} made me a ${flavor} cake.", default={"user":"Dennis"})
    >>> s.substitute(flavor="vanilla")
    'Dennis made me a vanilla cake.'
    >>> s.substitute(user="Ken", flavor="chocolate")
    'Ken made me chocolate cake.'

    @nitupho nitupho mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Oct 13, 2011
    @merwok
    Copy link
    Member

    merwok commented Oct 14, 2011

    Thanks for your interest in Python. Can you repost your code as a diff? (See the devguide for more info)

    @nitupho
    Copy link
    Mannequin Author

    nitupho mannequin commented Oct 14, 2011

    Here is my code as a diff

    @merwok
    Copy link
    Member

    merwok commented Oct 17, 2011

    Thanks for the patch. It mostly looks good; a detailed review follow.

    + The constructor takes two arguments, the first is the template string and
    + the second (optional) is a dictionary object which specify which values
    + should be used as default values, if no provided.
    Just say “a dictionary”, or even “a mapping”. There’s also a grammar typo and a bit of redundancy, so I propose: “is a mapping which gives default values”. What do you think about adding a small example in the examples section later in the file? It would probably be clearer than English.

           Like :meth:`substitute`, except that if placeholders are missing from
    -      *mapping* and *kwds*, instead of raising a :exc:`KeyError` exception, the
    -      original placeholder will appear in the resulting string intact.  Also,
    -      unlike with :meth:`substitute`, any other appearances of the ``$`` will
    -      simply return ``$`` instead of raising :exc:`ValueError`.
    +      *mapping*, *kwds* and *default*, instead of raising a :exc:`KeyError`
    default is not an argument, so *foo* markup is misleading.  Either use “the default values given to the constructor” or just “self.default”.

    + exception, the original placeholder will appear in the resulting string
    + intact. Also, unlike with :meth:`substitute`, any other appearances of
    + the ``$`` will simply return ``$`` instead of raising :exc:`ValueError`.
    If you don’t mind, I prefer if you have a few very short or too long lines if that helps reducing diff noise (i.e. lines that appear in the diff but are not changed, only rewrapped).

    +   .. attribute:: default
    +
    +      This is the optional dictionary object passed to the constructor's
    +      *template* argument.
    I’m not a native English speaker, but “passed to” seems wrong here (and in the other attribute’s doc).  I’d say “passed as the *default* argument”.
     
    -    def __init__(self, template):
    +    def __init__(self, template, default={}):
    Binding a mutable object to a local name at compile time is not good: All instances created without *default* argument will share the same dict, so editions to onetemplate.default will change anothertemplate.default too.  The common idiom is to use None as default value and add this:
            self.default = default if default is not None else {}

    @merwok
    Copy link
    Member

    merwok commented Oct 17, 2011

    Adding Georg, maintainer of the string module, to the nosy list. If he approves the idea, you can go ahead and complete your patch.

    @nitupho
    Copy link
    Mannequin Author

    nitupho mannequin commented Oct 18, 2011

    diff updated

    @nitupho
    Copy link
    Mannequin Author

    nitupho mannequin commented Oct 18, 2011

    A "light" version of diff added (rewrapped content is replaced by "[rewrapping]")

    @rhettinger
    Copy link
    Contributor

    This looks like a reasonable use case. That being said, I question whether the defaults should be attached directly to the template instance or whether they should be part of the substitution method.

    FWIW, there already have a couple of other ways to do it:

    >>> from string import Template
    >>> s = Template("${user} made me a ${flavor} cake.")
    >>> default = {"user":"Dennis"}
    >>> s.substitute(default, flavor='vanilla')
    'Dennis made me a vanilla cake.'
    >>> s.substitute(default, user='Ken', flavor='vanilla')
    'Ken made me a vanilla cake.'
     
    
    >>> from collections import ChainMap
    >>> s.substitute(ChainMap(dict(flavor='vanilla'), default))
    'Dennis made me a vanilla cake.'

    @rhettinger rhettinger self-assigned this Oct 18, 2011
    @nitupho
    Copy link
    Mannequin Author

    nitupho mannequin commented Oct 19, 2011

    When you are using a lot of string templates like I am doing, I think it's better if the defaults is attached directly to the template instance.

    This:
    [<Template0>, <Template1>, <Template2>, <Template3>, ...]
    is easier to use than:
    [(<Template0>, <defaults0>), (<Template1>, <defaults>1), ...]

    @rhettinger
    Copy link
    Contributor

    Barry, any thoughts?

    @rhettinger rhettinger assigned warsaw and unassigned rhettinger Oct 29, 2011
    @warsaw
    Copy link
    Member

    warsaw commented Nov 7, 2011

    When I need defaults, I make them part of the mapping that gets passed into .substitute() and .safe_substitute(). It doesn't feel to me like it's necessary to attach them to the Template instance. Also, couldn't you just subclass string.Template if you wanted defaults? OTOH, since it can be done in a backward compatible way, I guess I'm -0 on the change.

    @merwok
    Copy link
    Member

    merwok commented Jun 24, 2012

    It looks like a doc update to mention the excellent ChainMap class would be a good thing.

    Bfontaine, are you happy with using a Template subclass or changing your code slightly to have defaults outside of the Template objects, or do you still think the class should be changed? We could run this by the python-ideas mailing list.

    @serhiy-storchaka
    Copy link
    Member

    If the Template defaults should be attached directly to the template instance, then they should be attached directly to the string instance, for classic and modern string formatting. And this obviously is absurd.

    I agree with Éric, to mention ChainMap class in the string formatting documentation will be enough.

    @warsaw
    Copy link
    Member

    warsaw commented Nov 10, 2013

    I'm looking at this issue again with an eye toward Python 3.4.

    Raymond describes what I think is a reasonable way to use defaults:

    >>> x = Template('$foo $bar')
    >>> defaults = dict(foo='one', bar='two')
    >>> x.substitute(defaults)
    'one two'
    >>> x.substitute(defaults, bar='three')
    'one three'
    >>> x.substitute(defaults, foo='nine', bar='three')
    'nine three'

    (The implementation actually uses ChainMap.)

    Now, to address Bfontaine's complaint about passing around tuples, observe that Template instances are Just Instances, so you can always do this:

    >>> x = Template('$foo $bar')
    >>> x.defaults = defaults
    >>> x.substitute(x.defaults, foo='nine', bar='three')
    'nine three'

    IOW, just stash your defaults on the instance and pass the instance around. Does the Template class actually need more built-in support for defaults? I'm inclined to close this as Won't Fix.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants