[issue24685] collections.OrderedDict collaborative subclassing

[Raymond Hettinger]

Raymond Hettinger added the comment:

Sorry Eric, I'm going to mark this as rejected.  IMO this amounts to twisting 
the implementation into knots without any real payoff.  I'm glad you enjoyed my 
Pycon talk and were inspired by it.

--
resolution:  -> rejected
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24685] collections.OrderedDict collaborative subclassing

[Eric Frederich]

Eric Frederich added the comment:

I understand that in the general case you cannot just swap the order around and 
get the same behaviour.

This LoggingDict just prints some stuff to the screen and delegates to super 
and so I believe it should work wherever it is placed in a cooperative 
hierarchy.  Do you agree?

Now, I understand that OrderedDict is not cooperative.  You stated that this is 
a design decision and I respect that choice, but you also stated that classes 
can be made to be cooperative by creating a wrapper.

The reason I re-opened this bug is because I fail to see a way in which to 
create such a wrapper for Python3.  Do you believe that it should be possible 
to create a cooperative wrapper?

If it is possible (and its just my inability to create one) then I have no 
issue and the bug should be closed.
If it is not possible, then perhaps it could be noted somewhere that its not 
cooperative and impossible to make it cooperative and it should be listed last 
when using multiple inheritance.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24685
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24685] collections.OrderedDict collaborative subclassing

[Eric Frederich]

Eric Frederich added the comment:

Éric (Araujo),

Combinding defaultdict and OrderedDict is a little easier since one of them 
(defaultdict) has special behavior on getitem while the other (OrderedDict) has 
special behavior on setitem.

I played with mixing those two myself and saw some issues and found that I had 
to explicitly call __init__ on both base classes to get them primed properly.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24685
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24685] collections.OrderedDict collaborative subclassing

[Éric Araujo]

Éric Araujo added the comment:

FWIW I once helped a friend combine OrderedDict and defaultdict:
https://gist.github.com/merwok/11268759

The behavior can be surprising if we don’t know what Raymond said about design 
choices in OrderedDict, but it was not hard (in the default+ordered case) to 
work around.

--
nosy: +eric.araujo

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24685
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24685] collections.OrderedDict collaborative subclassing

[Raymond Hettinger]

Raymond Hettinger added the comment:

Eric, I believe you're profoundly misunderstanding my Pycon talk.  The posted 
code is not expected to work if I swap the order of the classes around -- 
MRO's are controlled by the child classes (subject to a number of constraints) 
and the order classes are listed matters greatly.  Also, cooperative classes 
have to be designed cooperatively.  The standard library OD class was 
intentionally tightly bound to its parent class to provide opportunities for a 
C implementation, to provide acceptable performance, and to make it easier for 
Jython and PyPy to implement the OD in ways favorable to their implementations. 
 Likewise it was intentional to hide access to the underlying dict using name 
mangling for the same reasons (that one was Guido's suggestion).  Also note, 
Guido's defaultdict also won't let you step in-between the class and its parent.

Also, your posted mal-functioning wrapper is mis-implemented.   Please see the 
How to Incorporate a Non-cooperative Class section of the referenced blog 
post.  The dependency injection technique is something that be used when 
testing your own subclasses which were built a loose coupling to a default 
supplier and where the calls between the consumer and supplier are a well 
defined part of the public API.  Only a handful of standard library tools were 
meant to be used this way. (That is why I the non-cooperative class integration 
part of the blog post was necessary).  

I've reclassified the tracker item as a feature request for 3.6.  Without 
compelling use cases, it will likely be re-rejected shortly. That said, I'm 
concerned that you've got a burr under your saddle and won't take no for an 
answer, so I'll leave this open for a while to allow you a chance to think 
about it and re-close it yourself.  I hope your will understand that we have 
valid reasons for not over-specifying the API and have a reluctance to tie our 
hands to the implementation details as the OD gets transitioned to a C 
implementation suitable internal use within the core.  FWIW, the OD code is 
pure python and there's nothing stopping you from using your own variants.

--
priority: normal - low
type:  - enhancement
versions:  -Python 3.5

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24685
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24685] collections.OrderedDict collaborative subclassing

[Eric Frederich]

Eric Frederich added the comment:

Raymond,

Thanks for the explanation of your reasoning.
Could you please provide an example of how to create a cooperative subclass of 
OrderedDict?

I have attempted to make one.
I succeeded to make it work where the previous example failed but in doing made 
the example that used to work now fail.
I have attached my attempt at making a cooperative OrderedDict in inj2.py

class coop_OrderedDict(OrderedDict):

A cooperative version of OrderedDict

def __setitem__(self, k, v, **kwargs):
# OrderedDict calls dict.__setitem__ directly skipping over LoggingDict
# fortunately we can control this with dict_setitem keyword argument

# calculate OrderedDict's real parent instead of skipping right to 
dict.__setitem__
# though depending on the hierarchy it may actually be dict.__setitem__
m = super(OrderedDict, self).__setitem__
# dict_setitem wants an unbound method
unbound_m = m.im_func
return super(coop_OrderedDict, self).__setitem__(k, v, 
dict_setitem=unbound_m)

In Python2 it fails with:

Traceback (most recent call last):
  File /tmp/inj2.py, line 51, in module
old1['hooray'] = 'it worked'
  File /tmp/inj2.py, line 30, in __setitem__
return super(LoggingDict, self).__setitem__(k, v)
  File /tmp/inj2.py, line 18, in __setitem__
unbound_m = m.im_func
AttributeError: 'method-wrapper' object has no attribute 'im_func'

In Python3 both cases fail.

--
Added file: http://bugs.python.org/file39998/inj2.py

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24685
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24685] collections.OrderedDict collaborative subclassing

[Eric Frederich]

Changes by Eric Frederich eric.freder...@gmail.com:


Removed file: http://bugs.python.org/file39998/inj2.py

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24685
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24685] collections.OrderedDict collaborative subclassing

[Eric Frederich]

Changes by Eric Frederich eric.freder...@gmail.com:


Added file: http://bugs.python.org/file3/inj2.py

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24685
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24685] collections.OrderedDict collaborative subclassing

[Eric Frederich]

Eric Frederich added the comment:

Attached, as inj3.py, is a version I made which seems to work with Python2 but 
not with Python3's C implementation of OrderedDict.

I had to walk the MRO myself to get the unbound method to pass along as 
dict_setitem.

With Python3 it doesn't look like doing this was left configurable.
It crashes complaining TypeError: wrapper __setitem__ doesn't take keyword 
arguments

Re-opening this bug since it seems impossible to make OrderedDict cooperative 
in Python3 even with a wrapper.

Perhaps Python3's OrderedDict should either
(a) be cooperative at the C level
(b) support dict_setitem keyword argument to maintain compatibility with 
Python2.

--
resolution: not a bug - 
status: closed - open
versions: +Python 3.5
Added file: http://bugs.python.org/file4/inj3.py

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24685
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24685] collections.OrderedDict collaborative subclassing

[Eric Frederich]

New submission from Eric Frederich:

After watching the PyCon talk Super considered super[1] and reading the 
corresponding blog post[2] I tried playing with dependency injection.

I was surprised to notice that the example he gave did not work if I swap the 
order of the classes around.  I think it should have.  See attached file.

I think this is a bug in collections.OrderedDict
OrderedDict is not well-behaved as far as cooperative subclassing is concerned.

The source code is hard wired with a keyword argument 
dict_setitem=dict.__setitem__ which it then calls at the end with 
dict_setitem(self, key, value)

A quick search of github for dict_setitem shows that this
bad practice seems be making its way into other projects

If dict_setitem keyword arg is really necessary to have, then maybe:

(a) have it default to None
(b) at the end of __setitem__ do something like:

if dict_setitem is not None:
return dict_setitem(self, key, value)

super(OrderedDict, self).__setitem__(key, value)

After a discussion on #py-dev this seemed like a reasonable request (not 
necessarily the implementation, but the idea that OrderedDict should cooperate).
I tested this against the C implementation of OrderedDict in Python 3.5 and 
noticed that it doesn't cooperate either.


[1] https://www.youtube.com/watch?v=EiOglTERPEo
[2] https://rhettinger.wordpress.com/2011/05/26/super-considered-super/

--
components: Library (Lib)
files: inj.py
messages: 247136
nosy: eric.frederich, eric.snow, rhettinger
priority: normal
severity: normal
status: open
title: collections.OrderedDict collaborative subclassing
versions: Python 2.7, Python 3.5
Added file: http://bugs.python.org/file39982/inj.py

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24685
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24685] collections.OrderedDict collaborative subclassing

[Raymond Hettinger]

Changes by Raymond Hettinger raymond.hettin...@gmail.com:


--
assignee:  - rhettinger
versions: +Python 3.6 -Python 2.7, Python 3.5

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24685
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24685] collections.OrderedDict collaborative subclassing

[Raymond Hettinger]

Raymond Hettinger added the comment:

This is an intentional design choice.  One reason for tightly coupling 
OrderedDict to dict was to preserve freedom for a C-implementation.  Another 
reason was for performance.  IIRC, using super() in __setitem__ slowed the OD 
from 10x slower than dicts to 20x.

Non-cooperative classes (of which Python has many) can be wrapped to make the 
classes cooperative.  The technique is discussed in the blog post 
https://rhettinger.wordpress.com/2011/05/26/super-considered-super/ .

--
resolution:  - not a bug
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24685
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com