Patches item #1433928, was opened at 2006-02-17 19:19 Message generated for change (Comment added) made by rhettinger You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1433928&group_id=5470
Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Core (C code) Group: Python 2.5 Status: Open >Resolution: Accepted Priority: 5 Submitted By: Guido van Rossum (gvanrossum) >Assigned to: Guido van Rossum (gvanrossum) Summary: add on_missing() and default_factory to dict Initial Comment: See the thread starting at http://mail.python.org/pipermail/python-dev/2006-February/061261.html This still needs unit tests and docs. ---------------------------------------------------------------------- >Comment By: Raymond Hettinger (rhettinger) Date: 2006-02-21 08:27 Message: Logged In: YES user_id=80475 Neal, the defdict_on_missing check for Py_None is necessary because the user can assign None to the factory and expect it to have the same effect as NULL. The other checks for NULL all automatically handle the Py_None case the same as if the attribute were filled with a real factory. Code nits: ---------- Pre-parse: PyEval_CallMethod((PyObject *)mp, "on_missing", "(O)", key); into: PyObject_CallMethodObjArgs((PyObject *)mp, "on_missing", key, NULL); The first four lines in defdict_dealloc() can be simplified to: Py_Clear(dd->default_factory); Likewise, the first five active lines of defdict_traverse shrink to: Py_VISIT(dd->default_factory); The OFF() macro is used only once. Better to in-line it. Other ----- If you don't get a chance to add the docs, just apply the patch and I'll write the docs later. Likewise, I'll update UserDict and DictMixin to keep the APIs in-sync. ---------------------------------------------------------------------- Comment By: Neal Norwitz (nnorwitz) Date: 2006-02-21 02:49 Message: Logged In: YES user_id=33168 In test_print() how about os.unlink(tfn)? Why do you play games with stdout, can't you do print >>f, ... ? (If you don't need stdout, it looks like you don't need sys.) In defdict_on_missing() why do you check if the factory is None? It's the only place None is checked for, everywhere else, it's just NULL. defdict_repr should be static. Type for this line in defdict_init() should be Py_ssize_t, not int: int n = PyTuple_GET_SIZE(args); ---------------------------------------------------------------------- Comment By: Guido van Rossum (gvanrossum) Date: 2006-02-20 20:53 Message: Logged In: YES user_id=6380 Here's a completely new version after another round of python-dev. - The built-in dict type still defines and calls on_missing(), but the default on_missing() implementation just raises KeyError(key). It no longer has a default_factory attribute. - You can subclass dict and override on_missing() to do whatever you want. - A useful subclass is collections.defaultdict; it defines an attribute default_factory and its on_missing() implementation calls that and inserts the resulting value in the dict (previous versions of the patch had this semantics in the built-in dict class, which was frowned upon). - Now with unit tests. - No docs yet, though. Assigning to Raymond Hettinger for review. Raymond, please assign it back to me for checkin if you're okay with this (or for revision if you're not :-). Because of Google's lawyers I must check this in myself. ---------------------------------------------------------------------- Comment By: Guido van Rossum (gvanrossum) Date: 2006-02-19 18:45 Message: Logged In: YES user_id=6380 Here's a version that doesn't crash in debug mode. Neal Norwitz is standing next to me and pointed out that I was attempting to decref something after tp_free had already wiped out the object. D'oh! ---------------------------------------------------------------------- Comment By: Guido van Rossum (gvanrossum) Date: 2006-02-19 14:00 Message: Logged In: YES user_id=6380 Sorry, forgot the upload. Here it is. ---------------------------------------------------------------------- Comment By: Guido van Rossum (gvanrossum) Date: 2006-02-19 13:57 Message: Logged In: YES user_id=6380 Aha. I'll have to try that. In the mean time, here's a new patch: - PyDict_GetItem is no longer involved - added {NULL} to PyMemberDef array - set ma_default_factory to NULL in both constructors Still no unit tests or docs. I have some misgivings about the API -- perhaps this should be a subclass. ---------------------------------------------------------------------- Comment By: Georg Brandl (birkenfeld) Date: 2006-02-19 10:39 Message: Logged In: YES user_id=1188172 Okay. I configured with "--with-pydebug" all the time, and then comes the segfault. Without "--with-pydebug", everything seems fine. ---------------------------------------------------------------------- Comment By: Guido van Rossum (gvanrossum) Date: 2006-02-19 10:28 Message: Logged In: YES user_id=6380 Are you sure you did a "make clean"? Because the dictobject struct lay-out is changed you may have to do that. If the problem persists, try adding setting ma_default_factory to NULL explicitly in dict_new (like it's already done in PyDict_New) and see if that makes a difference. BTW the change to PyDict_GetItem must be removed -- it's not a good idea. ---------------------------------------------------------------------- Comment By: Georg Brandl (birkenfeld) Date: 2006-02-18 05:39 Message: Logged In: YES user_id=1188172 Observations: * Doesn't the PyMemberDef need a sentinel? * Is PyObject_CallObject() faster than PyEval_CallFunction() with no arguments present? * The current patch gives me a segfault at interpreter exit because there is a dict object whose ma_default_factory was not initialized to NULL. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1433928&group_id=5470 _______________________________________________ Patches mailing list [email protected] http://mail.python.org/mailman/listinfo/patches
