1

Mypy considers this to be valid with strict = true:

from typing import Dict, TypeVar

KeyType = TypeVar("KeyType")
ValueType = TypeVar("ValueType")


class InvertibleDict(Dict[KeyType, ValueType]):
    def __inverse__(self) -> "InvertibleDict[ValueType, KeyType]":
        new_instance: "InvertibleDict[ValueType, KeyType]" = self.__class__()
        for key, value in self.items():
            new_instance[value] = key
        return new_instance

However, it does not accept the following, more concise version of the same code, saying that "Keywords must be strings" on the last line:

from typing import Dict, TypeVar

KeyType = TypeVar("KeyType")
ValueType = TypeVar("ValueType")


class InvertibleDict(Dict[KeyType, ValueType]):
    def __inverse__(self) -> "InvertibleDict[ValueType, KeyType]":
        return self.__class__(**{value: key for key, value in self.items()})
l0b0
  • 55,365
  • 30
  • 138
  • 223

2 Answers2

5

MyPy is correct here, it is catching a bug in your implementation (the beauty of static type checking). The type of:

{value: key for key, value in self.items()}

Is Dict[KeyType, ValueType], but that will fail in general when you do:

dict(**some_mapping)

Where the keys are not guaranteed to be strings.

Observe:

>>> dict(**{1:2,3:4})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: keywords must be strings

You just want:

return self.__class__({value: key for key, value in self.items()})

Which won't fail in general:

>>> dict({1:2,3:4})
{1: 2, 3: 4}

Personally, I would go with your first implementation regardless to not unnecessarily waste 2x the amount of space required, and do a needless second-pass.

Note, you would probably never use ** unpacking to initialize a dict, the keyword-argument form of the constructor is a convenience for writing something like:

>>> dict(foo=1, bar=2)
{'foo': 1, 'bar': 2}

You can even use this handy trick when copying a dictionary but wanting to force a value for particular string keys:

>>> dict({'foo': 1, 'bar': 2}, bar=42)
{'foo': 1, 'bar': 42}
juanpa.arrivillaga
  • 88,713
  • 10
  • 131
  • 172
  • Good point about the second pass, which I had not considered. This is exercise code, not production code, so clarity is most important, but I think I might go with this in any case. I'm not sure I understand why `dict({1:2,3:4})` works and `dict(**{1:2,3:4})` fails though. – l0b0 Jul 02 '21 at 03:53
  • 1
    @l0b0 because it is a *specialized form of the constructor* meant to work only with string keys. For whatever reason, it was designed to reject non-string keys when you use `**kwargs`, perhaps for consistency who knows. Again, you would really never do `dict(**some_other_dict)`, normally, to copy a built-in container you would just do `ContainerType(container_instance)`, e.g. `list(some_list)`, `set(some_set)`, `dict(some_dict)`.... – juanpa.arrivillaga Jul 02 '21 at 03:55
  • 1
    Interesting, thanks! [Implementation](https://gitlab.com/victor-engmark/mypy-exercises/-/merge_requests/27) for reference. – l0b0 Jul 02 '21 at 03:57
  • @l0b0 anyway, I don't think you gain any *clarity* by using the list comprehension. Concise isn't synonymous with clarity. That loop is about as clear and pythonic as it gets. I would just use `type(self)` but `self.__class__` is accetable – juanpa.arrivillaga Jul 02 '21 at 03:57
  • Thanks for the tip! I prefer `self.__class__()` to `type(self)()`, but it's good to know about alternatives. – l0b0 Jul 02 '21 at 04:00
0

Just for laughs I tried return self.__class__({value: key for key, value in self.items()}), which seems to work the same and passes mypy checks. TIL dicts can be initialised with a dict rather than **kwargs.

l0b0
  • 55,365
  • 30
  • 138
  • 223