string.Formatter doesn't support empty curly braces "{}" unlike str.format .

>>> import string
>>> a = string.Formatter()
>>> a.format("{}","test")
Traceback (most recent call last):
  File "<pyshell#2>", line 1, in <module>
    a.format("{}","hello")
  File "/usr/lib/python3.2/string.py", line 180, in format
    return self.vformat(format_string, args, kwargs)
  File "/usr/lib/python3.2/string.py", line 184, in vformat
    result = self._vformat(format_string, args, kwargs, used_args, 2)
  File "/usr/lib/python3.2/string.py", line 206, in _vformat
    obj, arg_used = self.get_field(field_name, args, kwargs)
  File "/usr/lib/python3.2/string.py", line 267, in get_field
    obj = self.get_value(first, args, kwargs)
  File "/usr/lib/python3.2/string.py", line 226, in get_value
    return kwargs[key]
KeyError: ''

Attached is patch to fix this issue.

Sorry, the patch has an mistake.
ValueErro should be ValueError.

Attached is a patch for test.test_string to test for this bug.
Can somebody please comment on my paches or commit my patches.

Why isn't anybody commiting or commenting on my patches?

This bug is assigned to me. Sometimes it takes a while before a committer has time to review a bug and act on it. I can assure you that I will review this before the next release of Python.

Thank you for the bug report, and especially thanks for the patch!

One thing that will definitely need to be developed before this is committed is one or more tests. Either you can add them, or I will before I commit the fix. There are some existing tests for str.format that can be leveraged for string.Formatter.

On Thu, Dec 15, 2011 at 1:42 AM, maniram maniram <report@bugs.python.org> wrote:

Why isn't anybody commiting or commenting on my patches?

Your patch is much appreciated. Thank you. It takes some time to get
things reviewed. Please read the Dev Guide. In particular, the
"Lifecycle of a Patch"
section: http://docs.python.org/devguide/patch.html.

Also, please quit marking issues as "languishing" unless the issue meets the
qualifications spelled out here:
http://docs.python.org/devguide/languishing.html.

test_string.diff looks good, except that it should probably only test the exception type, not the message (they are not a guaranteed part of the Python language and may change arbitrarily between versions or implementations (e.g. PyPy), so better not to add tests that depend on exact words).

I don’t have anything specific to say about bpo-13598.diff; if it makes the test pass, then it’s good. “if manual == True” should just be replaced by “if manual”.

If you’d like to, you can make one patch with fix + tests that addresses my comments and remove the older diffs.

One potential problem with the simple approach to fixing this is that up until now, string.Formatter has been thread safe. Because all the formatting state was held in local variables and passed around as method arguments, there was no state on the instance object to protect.

Now, this only applies if you start using the new feature, but it should be noted in the documentation and What's New that you need to limit yourself to accessing each formatter instance from a single thread.

It's also enough for me to say "no, not in a maintenance release".

Adding two attributes also seems unnecessary, and the pre-increment looks strange. Why not:

In __init__:
auto_field_count = 0

Then as the auto numbering checking, something like:

    auto_field_count = self.auto_field_count
    if field_name:
        if auto_field_count > 0:
           # Can't switch auto -> manual
        auto_field_count = -1
    elif auto_field_count < 0:
       # Can't switch manual -> auto
    else:
        field_name = str(auto_field_count)
        self.auto_field_count += 1

(Alternatively, I'd ask the question: why do we prevent mixing manual numbering and explicit numbering anyway? It's not like it's ambiguous at all)

@nick I don't understand why should my patch make Formatter thread-unsafe - the auto_field_count and manual variables are local variables just like the variables in the other functions in Formatter.

I have submitted a new patch, I have moved the increment to the end of if loop.

What is the status of the bug?

Could you upload just one patch with fix and test, addressing my previous comments, and remove the old patches? It will make it easier for Eric to review when he gets some time. Please also keep lines under 80 characters. Thanks in advance.

It seems like the patch doesn't consider mixing of positional and keyword arguments: if you have the format string "{foo} {} {bar}", then manual will be set to True when "foo" is seen as the field_name, and fail soon after when "" is seen as the field_name the next time around.

So, the test should include something which shows that

fmt.format("{foo} {} {bar}", 2, foo='fooval', bar='barval') returns "fooval 2 barval", whereas with a format string like "{foo} {0} {} {bar}" or "{foo} {} {0} {bar}" you get a ValueError.

Also, why "automatic field numbering" vs. "manual field specification"? Why not "numbering" for both?

Added a new patch which addresses Éric's comments.

Is there a reason "manual" is None, True, or False? Wouldn't just True or False suffice?

Is there a reason "manual" is None, True, or False? Wouldn't just True or False suffice?

I suppose before we see the first bracketed form ({} or {\d+}) we don't know which it is.

Yes, I guess that's so. I'll have to add a comment, as at first glance it just looks like a bug. Thanks!

Its not a bug though it has maintenance problems because if you change "manual is False" to not manual it no longer works correctly.

it has maintenance problems because if you change "manual is False"
to not manual it no longer works correctly.

So you should probably comment the initialisation appropriately.

One brief comment on the wording of the error message: the inconsistent naming is actually copied from the str.format code.

>>> "{foo} {} {bar}".format(2, foo='fooval', bar='barval')
'fooval 2 barval'
>>> "{foo} {0} {} {bar}".format(2, foo='fooval', bar='barval')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: cannot switch from manual field specification to automatic field numbering

The current patch fails to catch the fact that auto vs manual numbering has been used in following corner case:

from string import Formatter
print(Formatter().format("{0:{}}", 'foo', 5))

To fix this, without adding state to the formatter instance, some more information is going to need to be passed to the _vformat method.

Ramchandra's fix looks fairly good, although there is at least one remaining issue (see my last comment). I have attached a patch which addresses (and tests) this.

I'd be happy to pick this up if there are any remaining issues that need to be addressed, otherwise: I hope this helps.

Thanks,

Buump.

Looking at Phil Elson's patch:

I didn't see a test case relating to the example in his comment, namely

f.format("{0:{}}", 'foo', 5)

Did I miss it? The documentation also needs to be updated.

I didn't see a test case relating to the example in his comment, namely

f.format("{0:{}}", 'foo', 5)

Did I miss it?

The example should fail, which it wouldn't have done with the patch previously proposed. I believe the case is covered by the block:

    with self.assertRaises(ValueError):
        fmt.format("foo{1}{}", "bar", 6)

Though there is no harm in adding another test along the lines of:

    with self.assertRaises(ValueError):
        fmt.format("{0:{}}", "bar", 6)

If you think it is worthwhile?

I'm uncertain which documentation to update since the method which has had its signature updated is private and is called solely by Formatter.vformat .

Cheers,

I believe the case is covered by the block:
[snip]

Ah, right. I wasn't sure that was the exact same code path that was being exercised. But I didn't look very closely.

If you think it is worthwhile?

Only if it exercises a different code path.

New changeset ad74229a6fba by Eric V. Smith in branch '3.4':
Issue bpo-13598: Add auto-numbering of replacement fields to string.Formatter.
http://hg.python.org/cpython/rev/ad74229a6fba

New changeset 50fe497983fd by Eric V. Smith in branch '3.4':
Issue bpo-13598: Added acknowledgements to Misc/NEWS.
http://hg.python.org/cpython/rev/50fe497983fd

Is there any chance this will be fixed for 2.7 as well?