Issue 667730: More DictMixin - Python tracker (original) (raw)
Created on 2003-01-14 13:27 by s_keim, last changed 2022-04-10 16:06 by admin. This issue is now closed.
Messages (7)
Author: Sebastien Keim (s_keim)
Date: 2003-01-14 13:27
This patch is intended to provide a more consistent implementation for the various dictionary like objects of the standard library.
test_userdict has been rewritten, it now use unittest and define a test-case wich allow to check for conformity with the dictionary protocol.
test_shelve and test_weakref have been rewritten to use the test_userdict test-case.
test_os has been extended: a new test case check for environ object conformity to the dictionary protocol.
The patch modify the UserDict module:
- The doc says that contains should be one of the methods to redefine for better efficiency but the implementation make contains dependent of has_key definition. The patch reverse methods dependencies.
- Change iterkey = iter to def iterkey(self): return self.iter() to make iterkey able to use overiden iter methods.
- I have also a added init, copy and repr methods to DictMixin.
- The UserDict.UserDict class is a subclass of DictMixin, this allow to simplify UserDict implementation. The patch is rather conservative since a lot of methods definition could still be removed from UserDict.
In the weakref module, the patch make WeakValueDictionnary and WeakKeyDictionnary subclasses of UserDict.DictMixin. It also use nested scopes, the new generators syntax for iterator methods and rewrite WeakKeyDictionnary.delitem . All of this allow to decrease the module size by 50%.
In the shelve module, the patch add a copy() method which return a dictionary with the keys and values of the database.
Author: Martin v. Löwis (loewis) * 
Date: 2003-01-14 21:43
Logged In: YES user_id=21627
This patch breaks backwards compatibility. UserDict is an oldstyle class on purpose, since changing it to a newstyle class will certainly break the compatibility in subtle ways (e.g. by changing what type(userdictinstance) is).
Unless you can bring forward a better rationale than consistency, this patch will be rejected.
Author: Raymond Hettinger (rhettinger) *
Date: 2003-01-16 02:35
Logged In: YES user_id=80475
UserDict.UserDict should not change. As Martin pointed- out, inheriting from object changes the semantics in a non- backward compatible way. Also, the class is efficiently implemented in terms an internal dictionary and would be slowed down by the nest of calls in Mixin. Also, I think the code in incorrect in defining iter, there was a reason it was pulled out into a separate subclass -- that was done in Py2.2. and is not an easily reversible decision.
-0 on the changes to has_key() and contains(). has_key() was put at a lower level than contains because the older dict-style interfaces all define has_key.
+1 for changing iterkeys() to a full definition (and +1 for doing the same for iter()). Sabastien is correct is pointing out the advantages for propagating an overridden method.
-1 for altering repr() implementation. The current approach is shorter, cleaner, and faster.
-1 for adding nonzero(). Even dictionaries don't implement this method; they let len() do the talking.
-1 for adding init() and copy(). Both need to make assumptions about the order and number of parameters in the constructor of the class using the mixin. I think they are rarely helpful and are sometime harmful in introducing surprising, hard-to-find errors. People who need an init() or copy() can code them more cleanly and directly in the extending class. Also, I don't think the code is correct since DictMixin will be a base class, the use of super() is not what is wanted here -- if you were going to do this, try something like self.class(). Further, adding these methods violates my original intent for this class which was to extrapolate four basic mapping methods into a full mapping interface. It was not intended as a stand-alone class. Also, copy() cannot guarantee that it is copying all the relevant data for the sub-class and that violates the definition of what copy() is supposed to do. If something like this were attempted, it should be its own mixin (automatically adding copy support to any class) and it should be rather sophisticated about how to perfectly replicate itself (not easily done if the underlying data is in a file, database, or in a distributed app).
+0 on changing weakdicts provided it is done minimally and carefully with attention to leaving semantics unchanged and not slowing performance. The advantage goes beyond consistency, it removes code duplication, keeps well thought-out logic in one place, and provides an automatic interface update from DictMixin if the dictionary interface ever sprouts another method.
Author: Raymond Hettinger (rhettinger) *
Date: 2003-01-16 02:50
Logged In: YES user_id=80475
Also, +1 on consolidating the test cases though it should be done after any other changes to the files so we can make sure that nothing got broken.
Author: Sebastien Keim (s_keim)
Date: 2003-03-03 15:27
Logged In: YES user_id=498191
I have downloaded a new version of the patch updated to Python2.3a2
I hope to have removed all the stuff which could break backward compatibility since the new proposed patch contain now only the testing stuff (well, almost since I have also added a pop method to the weak dictionary classes to make them compatible with the test case).
Author: Raymond Hettinger (rhettinger) *
Date: 2003-03-07 05:52
Logged In: YES user_id=80475
The patch looks good. Please make two adjustments and re-submit.
Change the test_func docstrings to comment blocks. If a docstring is present, test support will print them in the summary instead of the test name.
Change the logic for mapping.pop() to accommodate the new default argument option which was added yesterday. The format is m.pop(key[, default]).
Author: Raymond Hettinger (rhettinger) *
Date: 2003-03-09 07:23
Logged In: YES user_id=80475
Accepted patch. Made the suggested fix-ups. Fixed spelling. Replace _tested_class method with an equivalent class variable.
Applied as: Lib/weakref.py 1.19 Lib/test/test_userdict.py 1.14 Lib/test/test_os.py 1.14 Lib/test/test_shelve.py 1.3 Lib/test/test_weakref.py 1.22