[Patches] [ python-Patches-547162 ] PEP 279 enumerate() implementation
noreply@sourceforge.net
noreply@sourceforge.net
Fri, 26 Apr 2002 12:41:49 -0700
Patches item #547162, was opened at 2002-04-22 12:49
You can respond by visiting:
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=547162&group_id=5470
Category: Core (C code)
Group: Python 2.3
Status: Open
>Resolution: Accepted
Priority: 5
Submitted By: Raymond Hettinger (rhettinger)
>Assigned to: Fred L. Drake, Jr. (fdrake)
Summary: PEP 279 enumerate() implementation
Initial Comment:
C version of enumerate().
Docs forthcoming this afternoon.
----------------------------------------------------------------------
>Comment By: Guido van Rossum (gvanrossum)
Date: 2002-04-26 15:41
Message:
Logged In: YES
user_id=6380
This is now checked in, except for the docs. I'm assigning
this to Fred for docs review and commit. Thanks, Raymond and
Alex!
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-04-25 08:46
Message:
Logged In: YES
user_id=6380
Thanks for asking! This is something that's not documented
very well...
en does have an ob_type member, it's part of the
PyObject_HEAD macro, so all objects have this. Since tp_free
in PyEnum_Type is initialized to PyObject_GC_Del, the effect
is the same as calling that function directly. The
indirection via ob_type->tp_free is so that a subclass can
override both tp_alloc and tp_free to manage memory for its
instances differently.
I'm not sure what you're asking about the XDECREF; the
XDECREF indeed takes care of the en_sit fiend, the tp_free
doesn't touch it.
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-04-25 00:41
Message:
Logged In: YES
user_id=80475
Please re-examine the recoding of enum_dealloc:
PyObject_GC_UnTrack(en);
Py_XDECREF(en->en_sit);
en->ob_type->tp_free(en);
The last line doesn't make sense to me.
en doesn't have an ob_type structure member.
I had expected: PyObject_GC_Del(en);
and the XDECREF ought to take care of the subordinate
iterator if its count drops to zero.
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-04-24 03:03
Message:
Logged In: YES
user_id=80475
Added a revised diff for the tutorial.
- Moved up one section to improve flow
- Retitled to Looping Techniques
- Added a zip() example
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-04-24 03:01
Message:
Logged In: YES
user_id=80475
Added a revised diff for the tutorial.
- Moved up one section to improve flow
- Retitled to Looping Techniques
- Added a zip() example
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-04-23 20:18
Message:
Logged In: YES
user_id=6380
Thanks for looking, Neil!
en_sit is NULL when the call to PyObject_GetIter(sit) in
enum_new() fails, and before this call completes. Since
GetIter can invoke arbitrary Python code, it's just possible
that it en might be traversed during this time. (Or is it
safe to call visit(NULL)? Then I'll remove the NULL test.)
I'll fix the nits in my copy before checking in, won't
bother regenerating the diff and uploading unless there are
other changes.
There's nothing to be done about enumerate(dict) yielding
the keys only, except point it out in a tutorial; doing
anything else would be too inconsistent to consider. We
crossed this bridge and burned it behind us when we (I :-)
decided that "for i in dict" should iterate over the keys.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-04-23 18:44
Message:
Logged In: YES
user_id=33168
This may be a stupid question, but how can en_sit ever be
NULL? In enum_new(), en_sit is guaranteed to be non-NULL.
But in enum_traverse(), en_sit is checked for NULL.
Other little things, inconsistent whitespace between
if & (. Docstring (enum_doc) is long, line should be wrapped.
While this probably isn't a big deal, enumerate(dict)
yields the keys only. This makes perfect sense to me,
but I don't know what newbies might expect.
All of these points are very minor though.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-04-23 16:33
Message:
Logged In: YES
user_id=6380
Here's a new combined diff, named enum2.diff, integrating
Raymond's changes and my changes, and incorporating Martin's
recommendations.
This is ready AFAIC, but I wouldn't mind a little more
review. Also, I'm not 100% sure on the name enumerate() --
today I like itemize() better. :-)
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-04-23 11:21
Message:
Logged In: YES
user_id=80475
Revised libfuncs.tex to include /versionadded{2.3}.
Revised test_enumerate.py to eliminate test of types.py.
Leaving GC changes for enumerate.c to GvR.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-04-23 09:58
Message:
Logged In: YES
user_id=6380
Yes, the GC API is used completely wrong, also mixing
old-style and new-style GC constants, and not using tp_alloc
/ tp_dealloc. I'm halfway fixing this, and adding support
for making the type subclassable.
enumobject.h is needed so that the type object can be
exported in the builtin module.
I agree that the types.py change shouldn't be done.
The C code should be indented with (8-space) tabs, not with
4 spaces.
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2002-04-23 03:10
Message:
Logged In: YES
user_id=21627
I recommend to add a test to trigger cyclic garbage, and a
garbage collection. The GC code looks wrong in multiple ways:
- it doesn't use GC_New
- it doesn't implement a tp_clear
See e.g. tupleobject.c for an example.
enumobject.h should get a include guard. It's not clear why
this is needed at all - if somebody needs it, you should add
the proper _Check macro as well.
The types.py change should be eliminated; I believe the
policy is to not extend types.py anymore.
In the Tex documentation, please use \versionadded, instead
of its expansion.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-04-22 16:19
Message:
Logged In: YES
user_id=6380
Thanks! I'll review it "soon". Tip for next time (don't
bother now): pack all the diffs in a single file. New files
can also be produced in diff form, by using diff -N. Having
6 separate files makes it 6 times as much work for a
reviewer to review the patch.
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-04-22 13:19
Message:
Logged In: YES
user_id=80475
Alex Martelli and I pair programmed this project -- meaning
he did the sophisticated part and then I picked at it and
then documented, tested, and packaged it.
----------------------------------------------------------------------
You can respond by visiting:
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=547162&group_id=5470