Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MutableMapping code smell (see OrderedDict) #49652

Closed
jimjjewett mannequin opened this issue Mar 2, 2009 · 2 comments
Closed

MutableMapping code smell (see OrderedDict) #49652

jimjjewett mannequin opened this issue Mar 2, 2009 · 2 comments
Assignees
Labels
stdlib Python modules in the Lib dir

Comments

@jimjjewett
Copy link
Mannequin

jimjjewett mannequin commented Mar 2, 2009

BPO 5402
Nosy @birkenfeld, @rhettinger, @pitrou, @mitsuhiko

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = 'https://github.com/rhettinger'
closed_at = <Date 2009-03-02.09:41:35.945>
created_at = <Date 2009-03-02.04:10:52.939>
labels = ['library']
title = 'MutableMapping code smell (see OrderedDict)'
updated_at = <Date 2009-03-02.09:41:51.608>
user = 'https://bugs.python.org/jimjjewett'

bugs.python.org fields:

activity = <Date 2009-03-02.09:41:51.608>
actor = 'rhettinger'
assignee = 'rhettinger'
closed = True
closed_date = <Date 2009-03-02.09:41:35.945>
closer = 'rhettinger'
components = ['Library (Lib)']
creation = <Date 2009-03-02.04:10:52.939>
creator = 'jimjjewett'
dependencies = []
files = []
hgrepos = []
issue_num = 5402
keywords = []
message_count = 2.0
messages = ['82999', '83010']
nosy_count = 5.0
nosy_names = ['georg.brandl', 'rhettinger', 'jimjjewett', 'pitrou', 'aronacher']
pr_nums = []
priority = 'low'
resolution = 'wont fix'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue5402'
versions = ['Python 3.1']

@jimjjewett
Copy link
Mannequin Author

jimjjewett mannequin commented Mar 2, 2009

Copy of bpo-5397

In python 3, UserDict (and DictMixin) are gone; presumably they should
be replaced by either a dict subclass or the ABC MutableMapping.

Unfortunately, there doesn't seem to be a clean way to inherit from
both.

In python 2.x, you could write

  class MyDict(DictMixin, dict): ...

but with

  class MyDict(MutableMapping, dict): ...

MutableMapping explicitly overrides __getitem__, __setitem__, and
__delitem__ to raise a KeyError.

The OrderedDict implementation in bpo-5397 puts the concrete class
first, and then handles composite methods manually, by either rewriting
them (repr, copy) or explicitly delegating past dict and straight to
MutableMapping (setdefault, update, etc.)

Both solutions seem fragile.

Unfortunately, the closest I come to a solution is to split the ABC
into a Mixin portion and an ABC enforcer.

    # The concrete methods of the current ABC
    class MapMixin: 

    # The abstract methods that get checked
    class MutableMapping(MapMixin):

# Trust that dict will always implement 
# all required concrete methods for us
class MyDict(MapMixin, dict):

@jimjjewett jimjjewett mannequin added the stdlib Python modules in the Lib dir label Mar 2, 2009
@rhettinger rhettinger self-assigned this Mar 2, 2009
@rhettinger
Copy link
Contributor

FWIW, I'm happy with Guido's design of MutableMapping. It parallels all
the other ABCs in its design and it succeeds completely at its primary
role as a defining an interface. Its secondary role is to provide some
mixin capability. It does well in this role when used as documented
(just filling-in the abstract methods as letting the mixin do the rest).

The case with OrderedDict is not a typical use of mixins because the
primary class (dict) already provides all of methods demanded by the
interface. I explicitly overwrite some of dict's methods because that
is part of the behavior that OrderedDict wants to define differently
than dict. It is appropriate that the primary class gets first dibs on
defining a method and that intentional overrides are done explicitly (as
they are in the OrderedDict example).

With OrderedDict, the only reason we subclassed from dict was to provide
interoperability with third-party tools that may have been hardwired to
work only with dicts. In general, the preferred approach is to not
subclass both dict and OrderedDict and to let the MutableMapping
interface do its job.

The proposed splitting of MutableMapping looks unhealthy and overly
complex to me. It makes ABCs harder to use in the general case just to
make one special case look a little prettier and do more of its magic
implicitly.

If this *really* bugs you, OrderedDict doesn't have to inherit from
MutableMapping at all. We can throw code reuse out the window and just
duplicate the relevant code fragments. To my eyes, either way is an
explicit override of the dict's methods which is the intended effect.

FWIW, the UserDict class in Py3.x was relocated to the collections
module. It isn't gone. We made an effort to kill it but found that
there were compelling use cases that were not a cleanly solved by any
other approach.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir
Projects
None yet
Development

No branches or pull requests

1 participant