classification
Title: object.__format__ should reject format strings
Type: behavior Stage: committed/rejected
Components: Interpreter Core Versions: Python 3.2, Python 2.7
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: eric.smith Nosy List: eric.smith, ezio.melotti, flox, mark.dickinson, meador.inge
Priority: normal Keywords: patch

Created on 2010-02-22 21:58 by eric.smith, last changed 2010-09-14 17:39 by eric.smith. This issue is now closed.

Files
File name Uploaded Description Edit
issue7994-2.diff eric.smith, 2010-02-23 19:47
issue7994-3.diff meador.inge, 2010-02-26 04:03 Updated patch off of trunk
Messages (14)
msg99847 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-02-22 21:58
Background:

format(obj, fmt) eventually calls object.__format__(obj, fmt) if obj (or one of its bases) does not implement __format__. The behavior of object.__format__ is basically:

def __format__(self, fmt):
    return str(self).__format__(fmt)

So the caller of format() thought they were passing in a format string specific to obj, but it is interpreted as a format string for str.

This is not correct, or at least confusing. The format string is supposed to be type specific. However in this case the object is being changed (to type str), but the format string which was to be applied to its original type is now being passed to str.

This is an actual problem that occurred in the migration from 3.0 -> 3.1 and from 2.6 -> 2.7 with complex. In the earlier versions, complex did not have a __format__ method, but it does in the latter versions. So this code:
>>> format(1+1j, '10s')
'(1+1j)    '
worked in 2.6 and 3.0, but gives an error in 2.7 and 3.1:
>>> format(1+1j, '10s')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Unknown format code 's' for object of type 'complex'

Proposal:
object.__format__ should give an error if a non-empty format string is specified. In 2.7 and 3.2 make this a PendingDeprecationWarning, in 3.3 make it a DeprecationWarning, and in 3.4 make it an error.

Modify the documentation to make this behavior clear, and let the user know that if they want this behavior they should say:

format(str(obj), '10s')

or the equivalent:

"{0!s:10}".format(obj)

That is, the conversion to str should be explicit.
msg99916 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-02-23 13:59
Proposed patch attached. I need to add tests and docs.
msg99917 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-02-23 14:00
issue7994-0.diff is against trunk.
msg99943 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-02-23 18:08
This version of the patch adds support for classic classes and adds tests. Documentation still needs to be written.

Again, this diff is against trunk.

If anyone wants to review this, in particular the tests that exercise PendingDeprecationWarning, that would be great.
msg99948 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-02-23 19:47
Patch with Misc/NEWS.
msg100135 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2010-02-26 04:03
The patch looks reasonable.  I built on it with the following changes:

   1. Added some extra test cases to cover Unicode format strings, 
      since the code was changed to handle these as well.
   2. Changed test_builtin.py by 
      s/m[0].message.message/str(w[0].message)/, since 
      BaseException.message was deprecated in 2.6.

I also have the following general comments:

   1. PEP 3101 explicitly defines the string conversion for 
      object.__format__.  What is the rationale behind this?  Should
      we find out before making this change?
   2. I don't think the comments in 'abstract.c' and 'typeobject.c'
      explaining that the warning will eventually become an error are
      needed.  I think it would be better to open separate issues for
      these migration steps as they can be tracked easier and will be 
      more visible.
   3. test_unicode, test_str have cases that trigger the added 
      warning.  Should they be altered now or when (if) this becomes 
      an error?
msg100139 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-02-26 08:10
I haven't looked at the patch, but:

Thanks for the the additional tests. Missing unicode was definitely a mistake.

str(w[0].message) is an improvement.

The PEP is out of date in many respects. I think it's best to note that in the PEP and continue to keep the documentation up-to-date.

This issue already applies to 3.3, but my plan is to remove that and create a new issue when I close this one. But I'd still like to leave the comments in place.

I'm aware of the existing tests which trigger the warning. I think they should probably be removed, although I haven't really spent much time thinking about it.
msg101921 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-03-30 07:13
Meador: Your patch (-3) looks identical to mine (-2), unless I'm making some mistake. Could you check? I'd like to get this applied in the next few days, before 2.7b1.

Thanks!
msg101943 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2010-03-30 15:27
Hi Eric,

(-2) and (-3) are different.  The changes that I made, however, are pretty minor.  Also, they are all in 'test_builtin.py'.
msg102162 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-04-02 12:35
Committed in trunk in r79596. I'll leave this open until I port to py3k, check the old tests for this usage, and create the issue to make it a DeprecationWarning.
msg116271 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2010-09-13 01:54
This should be merged before 3.2 beta.
msg116290 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2010-09-13 08:24
now the PendingDeprecationWarnings are checked in the test suite, with r84772 (for 2.7).
msg116350 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-09-13 20:51
Manually merged to py3k in r84790. I'll leave this open until I create the 3.3 issue to change it to a DeprecationWarning.
msg116414 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-09-14 17:39
See issue 9856 for changing this to a DeprecationWarning in 3.3.
History
Date User Action Args
2010-09-14 17:39:15eric.smithsetstatus: open -> closed

messages: + msg116414
2010-09-13 20:51:53eric.smithsetkeywords: - needs review

messages: + msg116350
versions: - Python 3.3
2010-09-13 08:24:14floxsetmessages: + msg116290
2010-09-13 01:54:20floxsetnosy: + flox
resolution: accepted
messages: + msg116271
2010-08-07 02:42:41ezio.melottisetnosy: + ezio.melotti
2010-04-02 12:35:39eric.smithsetmessages: + msg102162
stage: patch review -> committed/rejected
2010-03-30 15:27:10meador.ingesetmessages: + msg101943
2010-03-30 07:13:16eric.smithsetmessages: + msg101921
2010-02-26 08:10:48eric.smithsetmessages: + msg100139
2010-02-26 04:03:19meador.ingesetfiles: + issue7994-3.diff
nosy: + meador.inge
messages: + msg100135

2010-02-23 19:47:32eric.smithsetfiles: - issue7994-1.diff
2010-02-23 19:47:27eric.smithsetfiles: - issue7994-0.diff
2010-02-23 19:47:16eric.smithsetfiles: + issue7994-2.diff
2010-02-23 19:47:05eric.smithsetmessages: + msg99948
2010-02-23 18:47:18mark.dickinsonsetnosy: + mark.dickinson
2010-02-23 18:08:46eric.smithsetkeywords: - easy
files: + issue7994-1.diff
messages: + msg99943
2010-02-23 14:00:41eric.smithsetkeywords: + easy, needs review

messages: + msg99917
2010-02-23 13:59:54eric.smithsetstage: needs patch -> patch review
2010-02-23 13:59:37eric.smithsetfiles: + issue7994-0.diff
keywords: + patch
2010-02-23 13:59:15eric.smithsetmessages: + msg99916
2010-02-22 21:59:55eric.smithsetversions: + Python 3.3
2010-02-22 21:58:57eric.smithcreate