This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2010年04月16日 01:28 by Arfrever, last changed 2022年04月11日 14:56 by admin. This issue is now closed.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
structseq-inheritance.patch | benjamin.peterson, 2010年07月07日 00:29 | |||
structseq-inheritance2.patch | benjamin.peterson, 2010年07月07日 20:19 |
Messages (23) | |||
---|---|---|---|
msg103282 - (view) | Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) | Date: 2010年04月16日 01:28 | |
String interpolation doesn't work with sys.version_info in Python versions in which sys.version_info is a named tuple. It's a regression in Python 2.7 and 3.1. This problem doesn't concern named tuples created using collections.namedtuple(). This problem might also concern sys.getwindowsversion(), but I don't have access to Windows system, so I can't check it. $ python2.6 -c 'import sys; print("%s.%s.%s-%s-%s" % sys.version_info)' 2.6.5-final-0 $ python2.7 -c 'import sys; print("%s.%s.%s-%s-%s" % sys.version_info)' Traceback (most recent call last): File "<string>", line 1, in <module> TypeError: not enough arguments for format string $ python3.0 -c 'import sys; print("%s.%s.%s-%s-%s" % sys.version_info)' 3.0.1-final-0 $ python3.1 -c 'import sys; print("%s.%s.%s-%s-%s" % sys.version_info)' Traceback (most recent call last): File "<string>", line 1, in <module> TypeError: not enough arguments for format string |
|||
msg103305 - (view) | Author: Eric V. Smith (eric.smith) * (Python committer) | Date: 2010年04月16日 08:49 | |
This affects any type implemented as PyStructSequence. For example, sys.flags has the same behavior. |
|||
msg103797 - (view) | Author: Eric V. Smith (eric.smith) * (Python committer) | Date: 2010年04月21日 00:21 | |
I think the right way to handle this is to modify the test: if (Py_TYPE(args)->tp_as_mapping && !PyTuple_Check(args) && !PyObject_TypeCheck(args, &PyBaseString_Type)) to also exclude PyStructSequence's, but since they're all distinct types I don't see how to do that. I don't really think listing all of the PyStructSequence types would be acceptable. This is the test that's trying to figure out if %-formatting should use mapping or tuple expansion. |
|||
msg104519 - (view) | Author: Alexander Belopolsky (Alexander.Belopolsky) | Date: 2010年04月29日 15:18 | |
I am not sure fixing this quirk of %-formatting is worth the trouble given that one simply use new style {}-formatting: $ python-c 'import sys; print("{}.{}.{}-{}-{}".format(*sys.version_info))' 3.2.0-alpha-0 or a work-around $ python -c 'import sys; print("%s.%s.%s-%s-%s" % tuple(sys.version_info))' 3.2.0-alpha-0 |
|||
msg104521 - (view) | Author: Antoine Pitrou (pitrou) * (Python committer) | Date: 2010年04月29日 15:22 | |
Well, the problem is that it's a regression from 2.6. It also shows why these "fancy" structs aren't necessarily a good idea. I agree fixing in py3k is less important, though it would be nice too. |
|||
msg104523 - (view) | Author: Eric V. Smith (eric.smith) * (Python committer) | Date: 2010年04月29日 15:36 | |
The other thing to do would be to convince PyTuple_Check that PyStructSequences are tuples, but that would no doubt come with its own set of regressions. Since this problem is 1) hard to fix and 2) probably of minimal impact, I think the best course is to do nothing. |
|||
msg104524 - (view) | Author: Alexander Belopolsky (Alexander.Belopolsky) | Date: 2010年04月29日 15:37 | |
The recommended way to check that an object is an instance of a namedtuple is to check for the _fields attribute. See issue7796, msg99869. |
|||
msg104525 - (view) | Author: Eric V. Smith (eric.smith) * (Python committer) | Date: 2010年04月29日 15:43 | |
Yes, but sys.version_info isn't a namedtuple (which are in fact tuples), it's the (sort-of) C equivalent, which isn't a real tuple. >>> from collections import namedtuple >>> x = namedtuple('x', 'a b c') >>> '%s %s %s' % x(1, 2, 3) '1 2 3' Hmm, but sys.version_info is a tuple: >>> isinstance(sys.version_info, tuple) True I'll have to check if PyTuple_Check returns true or not. Maybe the problem is in the mapping check. I'll investigate more. |
|||
msg104529 - (view) | Author: Raymond Hettinger (rhettinger) * (Python committer) | Date: 2010年04月29日 15:52 | |
IIRC, there is an open ticket to make structseq inherit from tuple to avoid just this kind of problem. |
|||
msg104530 - (view) | Author: Eric V. Smith (eric.smith) * (Python committer) | Date: 2010年04月29日 15:56 | |
Looks like that's issue 1820. |
|||
msg104531 - (view) | Author: Alexander Belopolsky (Alexander.Belopolsky) | Date: 2010年04月29日 15:56 | |
On Thu, Apr 29, 2010 at 11:52 AM, Raymond Hettinger <report@bugs.python.org> wrote: .. > IIRC, there is an open ticket to make structseq inherit from tuple to avoid just this kind of problem. > Indeed, see issue1820. |
|||
msg108497 - (view) | Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) | Date: 2010年06月24日 01:13 | |
Should this regression block final release of 2.7 or can it be fixed in e.g. 2.7.1? |
|||
msg109445 - (view) | Author: Benjamin Peterson (benjamin.peterson) * (Python committer) | Date: 2010年07月07日 00:29 | |
Here's a patch. It makes structseq a subclass of tuple and along the way deletes tons of code. Please review. |
|||
msg109495 - (view) | Author: Alexander Belopolsky (belopolsky) * (Python committer) | Date: 2010年07月07日 19:56 | |
This is definitely the right way to do it. I expect that I will have only minor nit-picks as I go through the patch. 1. You can probably just do #define PyStructSequence_SET_ITEM PyTuple_SET_ITEM #define PyStructSequence_GET_ITEM PyTuple_GET_ITEM there is no need to repeat the argument lists. 2. I am comparing PyStructSequence_New and PyTuple_New: - PyStructSequence_New does not fill ob_item array with NULLs. - PyStructSequence_New does not call _PyObject_GC_TRACK I believe tp_free gets inherited, so structseq tp_new should follow what tuple's tp_new does. I am not 100% sure on the second point, though _PyObject_GC_TRACK may be redundant after PyObject_GC_NewVar. 3. In structseq_repr, do you need PyTuple_GetItem? I think you can get away with PyTuple_GET_ITEM, or better PyStructSequence_GET_ITEM. |
|||
msg109496 - (view) | Author: Benjamin Peterson (benjamin.peterson) * (Python committer) | Date: 2010年07月07日 20:19 | |
2010年7月7日 Alexander Belopolsky <report@bugs.python.org>: > > Alexander Belopolsky <belopolsky@users.sourceforge.net> added the comment: > > This is definitely the right way to do it. I expect that I will have only minor nit-picks as I go through the patch. > > 1. You can probably just do > > #define PyStructSequence_SET_ITEM PyTuple_SET_ITEM > #define PyStructSequence_GET_ITEM PyTuple_GET_ITEM > > there is no need to repeat the argument lists. I think the preprocessor requires this. (Anyway it fails without.) > > 2. I am comparing PyStructSequence_New and PyTuple_New: > - PyStructSequence_New does not fill ob_item array with NULLs. I'm not sure it really matters, since the fields should always be filled in, but I suppose it can't hurt... > - PyStructSequence_New does not call _PyObject_GC_TRACK > > I believe tp_free gets inherited, so structseq tp_new should follow what tuple's tp_new does. I am not 100% sure on the second point, though _PyObject_GC_TRACK may be redundant after PyObject_GC_NewVar. This is only needed when doing freelist magic in tupleobject.c. > > 3. In structseq_repr, do you need PyTuple_GetItem? I think you can get away with PyTuple_GET_ITEM, or better PyStructSequence_GET_ITEM. Yeah, that's fine. |
|||
msg109497 - (view) | Author: Alexander Belopolsky (belopolsky) * (Python committer) | Date: 2010年07月07日 20:28 | |
On Wed, Jul 7, 2010 at 4:19 PM, Benjamin Peterson <report@bugs.python.org> wrote: >> - PyStructSequence_New does not fill ob_item array with NULLs. > > I'm not sure it really matters, since the fields should always be > filled in, but I suppose it can't hurt... The following pattern is quite common: tuple = PyTuple_New(size): for (i = 0; i < size; ++i) { item = create_item(...); if (item == NULL) goto fail; } ... fail: Py_DECREF(tuple); I don't think this should be outlawed. |
|||
msg109498 - (view) | Author: Benjamin Peterson (benjamin.peterson) * (Python committer) | Date: 2010年07月07日 20:30 | |
(New patch addresses review.) |
|||
msg109499 - (view) | Author: Alexander Belopolsky (belopolsky) * (Python committer) | Date: 2010年07月07日 20:43 | |
Looks great. I love the patches where few '+'s are drowning in a sea of '-'s! A gratuitous nitpick (feel free to ignore). In + val = PyStructSequence_GET_ITEM(obj, i); if (cname == NULL || val == NULL) { The null check for val is now redundant, but won't hurt. |
|||
msg109500 - (view) | Author: Benjamin Peterson (benjamin.peterson) * (Python committer) | Date: 2010年07月07日 20:54 | |
Applied in r82636. |
|||
msg109501 - (view) | Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) | Date: 2010年07月07日 21:02 | |
I would suggest to document changed behavior of string interpolation with sys.version_info and sys.getwindowsversion() in "What’s New in Python 2.7" page and maybe "What’s New In Python 3.1" page. |
|||
msg109502 - (view) | Author: Raymond Hettinger (rhettinger) * (Python committer) | Date: 2010年07月07日 21:20 | |
This was a fantastic check-in. Thanks Benjamin. |
|||
msg109556 - (view) | Author: Brian Curtin (brian.curtin) * (Python committer) | Date: 2010年07月08日 17:22 | |
A side effect of this change is that it kills the ability to have a PyStructSequence which has a smaller visible size than the total number of items. For example, sys.getwindowsversion used to have 5 items in the sequence and 4 items accessible by name only (for backwards compatibility) -- now it's a 9 item tuple. |
|||
msg109577 - (view) | Author: Alexander Belopolsky (belopolsky) * (Python committer) | Date: 2010年07月08日 19:33 | |
> A side effect of this change is that it kills the ability to have a > PyStructSequence which has a smaller visible size than the total > number of items. Hmm. I was looking for this precise issue when I was reviewing the code and it appeared right to me. I know, I should have tested instead. This should be easy to fix, though. I'll look into it now. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022年04月11日 14:56:59 | admin | set | github: 52660 |
2010年07月08日 19:33:03 | belopolsky | set | messages: + msg109577 |
2010年07月08日 17:22:43 | brian.curtin | set | nosy:
+ brian.curtin messages: + msg109556 |
2010年07月07日 21:20:08 | rhettinger | set | messages: + msg109502 |
2010年07月07日 21:08:35 | eric.araujo | set | stage: test needed -> resolved |
2010年07月07日 21:02:29 | Arfrever | set | messages: + msg109501 |
2010年07月07日 20:54:27 | benjamin.peterson | set | status: open -> closed resolution: fixed messages: + msg109500 |
2010年07月07日 20:43:28 | belopolsky | set | messages: + msg109499 |
2010年07月07日 20:30:37 | benjamin.peterson | set | messages: + msg109498 |
2010年07月07日 20:28:03 | belopolsky | set | messages: + msg109497 |
2010年07月07日 20:19:37 | benjamin.peterson | set | files: + structseq-inheritance2.patch |
2010年07月07日 20:19:00 | benjamin.peterson | set | messages: + msg109496 |
2010年07月07日 19:56:26 | belopolsky | set | nosy:
+ belopolsky, - Alexander.Belopolsky messages: + msg109495 |
2010年07月07日 19:25:27 | belopolsky | link | issue5907 dependencies |
2010年07月07日 00:30:02 | benjamin.peterson | set | files:
+ structseq-inheritance.patch assignee: eric.smith -> benjamin.peterson messages: + msg109445 keywords: + patch |
2010年06月24日 01:13:17 | Arfrever | set | nosy:
+ benjamin.peterson messages: + msg108497 |
2010年04月29日 15:56:29 | Alexander.Belopolsky | set | messages: + msg104531 |
2010年04月29日 15:56:19 | eric.smith | set | messages: + msg104530 |
2010年04月29日 15:52:34 | rhettinger | set | messages: + msg104529 |
2010年04月29日 15:43:36 | eric.smith | set | messages: + msg104525 |
2010年04月29日 15:37:01 | Alexander.Belopolsky | set | messages: + msg104524 |
2010年04月29日 15:36:51 | eric.smith | set | messages: + msg104523 |
2010年04月29日 15:22:25 | pitrou | set | nosy:
+ pitrou messages: + msg104521 |
2010年04月29日 15:18:55 | Alexander.Belopolsky | set | nosy:
+ Alexander.Belopolsky messages: + msg104519 |
2010年04月21日 00:21:07 | eric.smith | set | messages: + msg103797 |
2010年04月16日 16:17:45 | rhettinger | set | priority: normal -> high |
2010年04月16日 12:05:00 | r.david.murray | set | nosy:
+ rhettinger |
2010年04月16日 10:57:27 | ezio.melotti | set | nosy:
+ ezio.melotti stage: test needed |
2010年04月16日 08:49:37 | eric.smith | set | priority: normal nosy: eric.smith, Arfrever messages: + msg103305 components: + Interpreter Core, - Library (Lib) type: behavior |
2010年04月16日 01:33:33 | benjamin.peterson | set | assignee: eric.smith nosy: + eric.smith |
2010年04月16日 01:28:08 | Arfrever | create |