[Patches] [ python-Patches-667730 ] More DictMixin

Mon, 03 Mar 2003 07:27:24 -0800

Patches item #667730, was opened at 2003-01-14 14:27
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=667730&group_id=5470

Category: Library (Lib)
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Sebastien Keim (s_keim)
Assigned to: Nobody/Anonymous (nobody)
Summary: More DictMixin

Initial Comment:
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.

----------------------------------------------------------------------

>Comment By: Sebastien Keim (s_keim)
Date: 2003-03-03 16:27

Message:
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).

----------------------------------------------------------------------

Comment By: Raymond Hettinger (rhettinger)
Date: 2003-01-16 03:50

Message:
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.

----------------------------------------------------------------------

Comment By: Raymond Hettinger (rhettinger)
Date: 2003-01-16 03:35

Message:
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.

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2003-01-14 22:43

Message:
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.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=667730&group_id=5470