Skip to content

typing.MutableMapping not a drop-in replacement for collections.abc.MutableMapping #203

Closed

Description

@bdarnell

The only difference between typing.MutableMapping and its collections.abc counterpart is supposed to be the support for generic type parameters. However, it looks like it's also missing some other methods:

import collections.abc
import typing

def define_user_dict(base_class):
    class MyUserDict(base_class):
        def __getitem__(self, k):
            return None
        def __setitem__(self, k, v):
            pass
        def __delitem__(self, k):
            pass
        def __iter__(self):
            return iter(())
        def __len__(self):
            return 0
    return MyUserDict

UserDict1 = define_user_dict(collections.abc.MutableMapping)
UserDict2 = define_user_dict(typing.MutableMapping[str, str])

UserDict1().update() # this one works
UserDict2().update() # this one fails
$ python3.5 /tmp/mappingtest.py
Traceback (most recent call last):
  File "/tmp/mappingtest.py", line 22, in <module>
    UserDict2().update()
AttributeError: 'MyUserDict' object has no attribute 'update'

Tested on python 3.5.1, and on python 3.4 with typing 3.5.1.0 installed.

Is changing my base class from collections.abc.MutableMapping to typing.MutableMapping the right way to tell the type checker what types my user dict will work with? Should I be using both base classes together? Is there a way to tell the type checker that MyUserDict is a typing.MutableMapping[str, str] while keeping collections.abc.MutableMapping as the runtime base class?

Additionally, isinstance() checking is funky: isinstance(dict(), UserDict2) returns true, so I can't use isinstance() to distinguish my dict type from any other.

Activity

  1. bintoro commented on Apr 17, 2016

    @bintoro
    Contributor

    The reason for this is that the typing ABCs don't really inherit from their collections.abc counterparts. They just proxy issubclass calls to the collections classes.

    Incidentally, I just today proposed changing this in a comment to #136.

    The branch is here: https://github.com/bintoro/typing/tree/extra-as-base

    Additionally, isinstance() checking is funky: isinstance(dict(), UserDict2) returns true, so I can't use isinstance() to distinguish my dict type from any other.

    I raised the same issue in #202. This is also fixed in the above branch.

  2. gvanrossum commented on Apr 18, 2016

    @gvanrossum
    Member

    Agreed this is a problem. But I'm not sure I am going to get to this before Python 3.5.2 goes out -- not that that's about to be released, but I need to prioritize and I don't think that this (or the other patches proposed by @bintoro) is more important than the work on typeshed and mypy, and we do have a few mypy releases planned (May 5 and May 26).

    Does combining both base classes work at all? If so it would be a reasonable work-around.

  3. bdarnell commented on Apr 18, 2016

    @bdarnell
    Author

    It's not a priority for me.

    The three different outcomes are:

    • collections.abc.MutableMapping alone: Everything works, but the code is less precisely typed than it could be. I'm sticking with this one for now.
    • typing.MutableMapping alone: Things break because methods like update() are missing. If it made it farther it would also see isinstance() problems.
    • Both base classes combined: Things mostly work but isinstance() is misbehaving and causing errors in some cases.
  4. modified the milestone: 3.5.3 on Aug 26, 2016
  5. added a commit that references this issue on Sep 27, 2016
  6. gvanrossum commented on Sep 27, 2016

    @gvanrossum
    Member

    Sorry, #283 only partially fixed this.

  7. gvanrossum commented on Sep 28, 2016

    @gvanrossum
    Member

    @ilevkivskyi Could you look into this more? The test from the first post still fails.

  8. ilevkivskyi commented on Sep 28, 2016

    @ilevkivskyi
    Member

    @gvanrossum The first question is whether we really want typing.MutableMapping to work in terms of implementation. It looks like specifying two bases works:

    class MD(collections.abc.MutableMapping, typing.MutableMapping[str, str]):
         def __getitem__(self, k):
             return None
         def __setitem__(self, k, v):
             pass
         def __delitem__(self, k):
             pass
         def __iter__(self):
             return iter(())
         def __len__(self):
             return 0
    
    >>> MD().update()  # This works
    >>> isinstance(MD(), collections.abc.MutableMapping)
    True
    >>> isinstance(MD(), typing.MutableMapping)
    True
    >>> isinstance(MD(), typing.Dict)
    False
    >>> isinstance(MD(), collections.abc.Set)
    False
    # All other possible instance checks also work fine

    But if you think that we should include the implementation (i.e. include collections.abc.MutableMapping in __mro__, not only in __subclasscheck__)
    then a possible solution would be similar to proposed in #207 (patch bases at generic class creation and define __subclasshook__).

  9. gvanrossum commented on Sep 28, 2016

    @gvanrossum
    Member
  10. bintoro commented on Sep 28, 2016

    @bintoro
    Contributor

    It should be pretty straightforward to take the parts from #207 that established functional equivalence between typing.X and collections.abc.X and plug them into the current version.

  11. ilevkivskyi commented on Sep 29, 2016

    @ilevkivskyi
    Member

    Here is a PR as proposed by @bintoro #287

  12. added a commit that references this issue on Sep 29, 2016
    83d4b71
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions