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.

classification
Title: More DictMixin
Type: Stage:
Components: Library (Lib) Versions:
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: loewis, rhettinger, s_keim
Priority: normal Keywords: patch

Created on 2003-01-14 13:27 by s_keim, last changed 2022-04-10 16:06 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
dico.diff s_keim, 2003-01-14 13:29 context diff
dico2.diff s_keim, 2003-03-03 15:27
Messages (7)
msg42412 - (view) 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.
msg42413 - (view) Author: Martin v. Löwis (loewis) * (Python committer) 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.
msg42414 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) 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.
msg42415 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) 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.
msg42416 - (view) 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).
msg42417 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2003-03-07 05:52
Logged In: YES 
user_id=80475

The patch looks good.
Please make two adjustments and re-submit.

1) 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.

2) Change the logic for mapping.pop() to accommodate 
the new default argument option which was added 
yesterday.  The format is m.pop(key[, default]).
msg42418 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) 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
History
Date User Action Args
2022-04-10 16:06:08adminsetgithub: 37768
2003-01-14 13:27:10s_keimcreate