classification
Title: string.Formatter doesn't support empty curly braces "{}"
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.smith Nosy List: Ramchandra Apte, aronacher, binkert, eric.araujo, eric.smith, ezio.melotti, flox, meador.inge, ncoghlan, pelson, python-dev, terry.reedy, vinay.sajip
Priority: normal Keywords: patch

Created on 2011-12-14 03:59 by Ramchandra Apte, last changed 2014-05-08 14:46 by aronacher. This issue is now closed.

Files
File name Uploaded Description Edit
issue13598.diff Ramchandra Apte, 2011-12-14 05:51 A patch to fix this issue (Version 2)
test_string.diff Ramchandra Apte, 2011-12-14 09:06 A patch to test.test_string to test for this bug.
issue13598.diff Ramchandra Apte, 2012-02-17 04:31 A patch to fix this issue (Version 3)
issue13598.diff Ramchandra Apte, 2012-06-04 10:27 A patch to fix this issue (Version 4)
test_string.diff Ramchandra Apte, 2012-06-04 10:28 A patch to test.test_string to test for this bug. (Version 2)
pelson_issue13598.diff pelson, 2012-10-31 14:22 An alternative patch to fix the issue (test included). (vn1) review
Messages (30)
msg149420 - (view) Author: Ramchandra Apte (Ramchandra Apte) * Date: 2011-12-14 03:59
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: ''
msg149424 - (view) Author: Ramchandra Apte (Ramchandra Apte) * Date: 2011-12-14 05:39
Attached is patch to fix this issue.
msg149426 - (view) Author: Ramchandra Apte (Ramchandra Apte) * Date: 2011-12-14 05:51
Sorry, the patch has an mistake.
ValueErro should be ValueError.
msg149433 - (view) Author: Ramchandra Apte (Ramchandra Apte) * Date: 2011-12-14 09:06
Attached is a patch for test.test_string to test for this bug.
Can somebody please comment on my paches or commit my patches.
msg149515 - (view) Author: Ramchandra Apte (Ramchandra Apte) * Date: 2011-12-15 07:42
Why isn't anybody commiting or commenting on my patches?
msg149523 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2011-12-15 10:15
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.
msg149566 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-12-15 17:26
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.
msg150507 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-01-03 16:16
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 issue13598.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.
msg153397 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-02-15 09:01
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)
msg153532 - (view) Author: Ramchandra Apte (Ramchandra Apte) * Date: 2012-02-17 04:07
@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.
msg153533 - (view) Author: Ramchandra Apte (Ramchandra Apte) * Date: 2012-02-17 04:31
I have submitted a new patch, I have moved the increment to the end of if loop.
msg162207 - (view) Author: Ramchandra Apte (Ramchandra Apte) * Date: 2012-06-03 12:06
What is the status of the bug?
msg162224 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-06-03 17:06
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.
msg162226 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2012-06-03 18:28
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?
msg162254 - (view) Author: Ramchandra Apte (Ramchandra Apte) * Date: 2012-06-04 10:27
Added a new patch which addresses Éric's comments.
msg162549 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2012-06-08 22:02
Is there a reason "manual" is None, True, or False? Wouldn't just True or False suffice?
msg162550 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2012-06-08 22:16
> 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.
msg162551 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2012-06-08 22:49
Yes, I guess that's so. I'll have to add a comment, as at first glance it just looks like a bug. Thanks!
msg162562 - (view) Author: Ramchandra Apte (Ramchandra Apte) * Date: 2012-06-09 14:51
Its not a bug though it has maintenance problems because if you change "manual is False" to not manual it no longer works correctly.
msg162565 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2012-06-09 21:26
> 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.
msg162668 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-06-12 11:57
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
msg174272 - (view) Author: Phil Elson (pelson) * Date: 2012-10-31 12:08
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.
msg174282 - (view) Author: Phil Elson (pelson) * Date: 2012-10-31 14:22
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,
msg180281 - (view) Author: Ramchandra Apte (Ramchandra Apte) * Date: 2013-01-20 05:43
Buump.
msg180290 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2013-01-20 13:03
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.
msg183434 - (view) Author: Phil Elson (pelson) * Date: 2013-03-04 12:09
> 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,
msg183453 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2013-03-04 13:48
> 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.
msg216207 - (view) Author: Roundup Robot (python-dev) Date: 2014-04-14 20:44
New changeset ad74229a6fba by Eric V. Smith in branch '3.4':
Issue #13598: Add auto-numbering of replacement fields to string.Formatter.
http://hg.python.org/cpython/rev/ad74229a6fba
msg216208 - (view) Author: Roundup Robot (python-dev) Date: 2014-04-14 20:47
New changeset 50fe497983fd by Eric V. Smith in branch '3.4':
Issue #13598: Added acknowledgements to Misc/NEWS.
http://hg.python.org/cpython/rev/50fe497983fd
msg218114 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2014-05-08 14:46
Is there any chance this will be fixed for 2.7 as well?
History
Date User Action Args
2014-05-08 14:46:39aronachersetnosy: + aronacher
messages: + msg218114
2014-04-14 20:47:00python-devsetmessages: + msg216208
2014-04-14 20:45:35eric.smithsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2014-04-14 20:44:00python-devsetnosy: + python-dev
messages: + msg216207
2014-04-14 17:12:34eric.smithsetversions: + Python 3.5, - Python 3.4
2013-03-04 13:48:58vinay.sajipsetmessages: + msg183453
2013-03-04 12:09:05pelsonsetmessages: + msg183434
2013-02-25 09:13:23eric.smithsetnosy: + binkert
2013-01-20 13:03:20vinay.sajipsetmessages: + msg180290
2013-01-20 05:43:22Ramchandra Aptesetmessages: + msg180281
2012-10-31 14:22:42pelsonsetfiles: + pelson_issue13598.diff

messages: + msg174282
2012-10-31 12:08:19pelsonsetmessages: + msg174272
2012-10-31 11:00:25pelsonsetnosy: + pelson
2012-10-10 04:10:21ezio.melottisetnosy: + ezio.melotti

versions: + Python 3.4, - Python 3.3
2012-06-12 11:57:54ncoghlansetmessages: + msg162668
2012-06-09 21:26:23vinay.sajipsetmessages: + msg162565
2012-06-09 14:51:02Ramchandra Aptesetmessages: + msg162562
2012-06-08 22:49:45eric.smithsetmessages: + msg162551
2012-06-08 22:16:42vinay.sajipsetmessages: + msg162550
2012-06-08 22:02:15eric.smithsetmessages: + msg162549
2012-06-04 10:28:40Ramchandra Aptesetfiles: + test_string.diff
2012-06-04 10:27:37Ramchandra Aptesetfiles: + issue13598.diff

messages: + msg162254
2012-06-03 23:16:38floxsetnosy: + flox
2012-06-03 18:28:12vinay.sajipsetnosy: + vinay.sajip
messages: + msg162226
2012-06-03 17:06:26eric.araujosetmessages: + msg162224
2012-06-03 12:06:45Ramchandra Aptesetmessages: + msg162207
2012-03-16 19:21:15terry.reedylinkissue14297 superseder
2012-02-17 04:31:19Ramchandra Aptesetfiles: + issue13598.diff

messages: + msg153533
2012-02-17 04:07:14Ramchandra Aptesetmessages: + msg153532
2012-02-15 09:01:49ncoghlansetnosy: + ncoghlan

messages: + msg153397
versions: - Python 2.7, Python 3.2
2012-01-03 16:16:05eric.araujosetnosy: + eric.araujo
messages: + msg150507
2011-12-17 00:22:17terry.reedysetnosy: + terry.reedy

versions: + Python 2.7
2011-12-15 17:26:53meador.ingesetnosy: + meador.inge
messages: + msg149566
2011-12-15 10:15:27eric.smithsetmessages: + msg149523
stage: patch review
2011-12-15 07:44:54Ramchandra Aptesetstatus: languishing -> open
2011-12-15 07:42:09Ramchandra Aptesetstatus: open -> languishing

messages: + msg149515
2011-12-14 16:33:08benjamin.petersonsetassignee: eric.smith

nosy: + eric.smith
2011-12-14 09:06:49Ramchandra Aptesetfiles: + test_string.diff

messages: + msg149433
2011-12-14 05:51:59Ramchandra Aptesetfiles: - issue13598.diff
2011-12-14 05:51:29Ramchandra Aptesetfiles: + issue13598.diff

messages: + msg149426
2011-12-14 05:40:30Ramchandra Aptesetcomponents: + Library (Lib)
versions: + Python 3.2, Python 3.3
2011-12-14 05:39:53Ramchandra Aptesetfiles: + issue13598.diff
type: behavior
messages: + msg149424

keywords: + patch
2011-12-14 03:59:21Ramchandra Aptecreate