Thread-safe Buffer.__items__; proper KeysView, ItemsView, ValuesView (#…

…93)

.coveragerc

@@ -0,0 +1,17 @@
+[run]
+source =
+ zict
+omit =
+
+[report]
+show_missing = True
+exclude_lines =
+ # re-enable the standard pragma
+ pragma: nocover
+ pragma: no cover
+ # always ignore type checking blocks
+ TYPE_CHECKING
+ @overload
+
+[html]
+directory = coverage_html_report

doc/source/changelog.rst

@@ -15,10 +15,12 @@ Changelog
 - ``LMDB`` now uses memory-mapped I/O on MacOSX and is usable on Windows.
  (:pr:`78`) `Guido Imperiale`_
 - The library is now partially thread-safe.
- (:pr:`82`, :pr:`90`) `Guido Imperiale`_
+ (:pr:`82`, :pr:`90`, :pr:`93`) `Guido Imperiale`_
 - :class:`LRU` and :class:`Buffer` now support delayed eviction.
  New objects :class:`Accumulator` and :class:`InsertionSortedSet`.
  (:pr:`87`) `Guido Imperiale`_
+- All mappings now return proper KeysView, ItemsView, and ValuesView objects from their
+ keys(), items(), and values() methods (:pr:`93`) `Guido Imperiale`_
 
 
 2.2.0 - 2022-04-28

zict/buffer.py

@@ -2,6 +2,10 @@
 
 from collections.abc import Callable, Iterator, MutableMapping
 from itertools import chain
+from typing import ( # TODO import from collections.abc (needs Python >=3.9)
+ ItemsView,
+ ValuesView,
+)
 
 from zict.common import KT, VT, ZictBase, close, flush
 from zict.lru import LRU
@@ -136,21 +140,30 @@ def __delitem__(self, key: KT) -> None:
  except KeyError:
  del self.slow[key]
 
- # FIXME dictionary views https://github.com/dask/zict/issues/61
- def keys(self) -> Iterator[KT]: # type: ignore
- return iter(self)
+ def values(self) -> ValuesView[VT]:
+ return BufferValuesView(self)
 
- def values(self) -> Iterator[VT]: # type: ignore
- return chain(self.fast.values(), self.slow.values())
-
- def items(self) -> Iterator[tuple[KT, VT]]: # type: ignore
- return chain(self.fast.items(), self.slow.items())
+ def items(self) -> ItemsView[KT, VT]:
+ return BufferItemsView(self)
 
  def __len__(self) -> int:
  return len(self.fast) + len(self.slow)
 
  def __iter__(self) -> Iterator[KT]:
- return chain(self.fast, self.slow)
+ """Make sure that the iteration is not disrupted if you evict/restore a key in
+ the middle of it
+ """
+ seen = set()
+ while True:
+ try:
+ for d in (self.fast, self.slow):
+ for key in d:
+ if key not in seen:
+ seen.add(key)
+ yield key
+ return
+ except RuntimeError:
+ pass
 
  def __contains__(self, key: object) -> bool:
  return key in self.fast or key in self.slow
@@ -165,3 +178,25 @@ def flush(self) -> None:
 
  def close(self) -> None:
  close(self.fast, self.slow)
+
+
+class BufferItemsView(ItemsView[KT, VT]):
+ _mapping: Buffer # FIXME CPython implementation detail
+ __slots__ = ()
+
+ def __iter__(self) -> Iterator[tuple[KT, VT]]:
+ # Avoid changing the LRU
+ return chain(self._mapping.fast.items(), self._mapping.slow.items())
+
+
+class BufferValuesView(ValuesView[VT]):
+ _mapping: Buffer # FIXME CPython implementation detail
+ __slots__ = ()
+
+ def __contains__(self, value: object) -> bool:
+ # Avoid changing the LRU
+ return any(value == v for v in self)
+
+ def __iter__(self) -> Iterator[VT]:
+ # Avoid changing the LRU
+ return chain(self._mapping.fast.values(), self._mapping.slow.values())

zict/cache.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 import weakref
-from collections.abc import Iterator, KeysView, MutableMapping
+from collections.abc import Iterator, MutableMapping
 from typing import TYPE_CHECKING
 
 from zict.common import KT, VT, ZictBase, close, flush
@@ -67,7 +67,6 @@ def __getitem__(self, key: KT) -> VT:
  def __setitem__(self, key: KT, value: VT) -> None:
  # If the item was already in cache and data.__setitem__ fails, e.g. because it's
  # a File and the disk is full, make sure that the cache is invalidated.
- # FIXME https://github.com/python/mypy/issues/10152
  try:
  del self.cache[key]
  except KeyError:
@@ -94,11 +93,6 @@ def __contains__(self, key: object) -> bool:
  # Do not let MutableMapping call self.data[key]
  return key in self.data
 
- def keys(self) -> KeysView[KT]:
- # Return a potentially optimized set-like, instead of letting MutableMapping
- # build it from __iter__ on the fly
- return self.data.keys()
-
  def flush(self) -> None:
  flush(self.cache, self.data)
 

zict/file.py

@@ -3,7 +3,7 @@
 import mmap
 import os
 import pathlib
-from collections.abc import Iterator, KeysView
+from collections.abc import Iterator
 from urllib.parse import quote, unquote
 
 from zict.common import ZictBase
@@ -139,9 +139,6 @@ def __setitem__(
  def __contains__(self, key: object) -> bool:
  return key in self.filenames
 
- def keys(self) -> KeysView[str]:
- return self.filenames.keys()
-
  def __iter__(self) -> Iterator[str]:
  return iter(self.filenames)
 

zict/func.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from collections.abc import Callable, Iterable, Iterator, KeysView, MutableMapping
+from collections.abc import Callable, Iterable, Iterator, MutableMapping
 from typing import Generic, TypeVar
 
 from zict.common import KT, VT, ZictBase, close, flush
@@ -71,16 +71,6 @@ def __contains__(self, key: object) -> bool:
  def __delitem__(self, key: KT) -> None:
  del self.d[key]
 
- def keys(self) -> KeysView[KT]:
- return self.d.keys()
-
- # FIXME dictionary views https://github.com/dask/zict/issues/61
- def values(self) -> Iterator[VT]: # type: ignore
- return (self.load(v) for v in self.d.values())
-
- def items(self) -> Iterator[tuple[KT, VT]]: # type: ignore
- return ((k, self.load(v)) for k, v in self.d.items())
-
  def _do_update(self, items: Iterable[tuple[KT, VT]]) -> None:
  it = ((k, self.dump(v)) for k, v in items)
  self.d.update(it)

zict/lmdb.py

@@ -3,6 +3,10 @@
 import pathlib
 import sys
 from collections.abc import Iterable, Iterator
+from typing import ( # TODO import from collections.abc (needs Python >=3.9)
+ ItemsView,
+ ValuesView,
+)
 
 from zict.common import ZictBase
 
@@ -77,18 +81,15 @@ def __contains__(self, key: object) -> bool:
  with self.db.begin() as txn:
  return txn.cursor().set_key(_encode_key(key))
 
- # FIXME dictionary views https://github.com/dask/zict/issues/61
- def items(self) -> Iterator[tuple[str, bytes]]: # type: ignore
- cursor = self.db.begin().cursor()
- return ((_decode_key(k), v) for k, v in cursor.iternext(keys=True, values=True))
-
- def keys(self) -> Iterator[str]: # type: ignore
+ def __iter__(self) -> Iterator[str]:
  cursor = self.db.begin().cursor()
  return (_decode_key(k) for k in cursor.iternext(keys=True, values=False))
 
- def values(self) -> Iterator[bytes]: # type: ignore
- cursor = self.db.begin().cursor()
- return cursor.iternext(keys=False, values=True)
+ def items(self) -> ItemsView[str, bytes]:
+ return LMDBItemsView(self)
+
+ def values(self) -> ValuesView[bytes]:
+ return LMDBValuesView(self)
 
  def _do_update(self, items: Iterable[tuple[str, bytes]]) -> None:
  # Optimized version of update() using a single putmulti() call.
@@ -97,9 +98,6 @@ def _do_update(self, items: Iterable[tuple[str, bytes]]) -> None:
  consumed, added = txn.cursor().putmulti(items_enc)
  assert consumed == added == len(items_enc)
 
- def __iter__(self) -> Iterator[str]:
- return self.keys()
-
  def __delitem__(self, key: str) -> None:
  with self.db.begin(write=True) as txn:
  if not txn.delete(_encode_key(key)):
@@ -110,3 +108,35 @@ def __len__(self) -> int:
 
  def close(self) -> None:
  self.db.close()
+
+
+class LMDBItemsView(ItemsView[str, bytes]):
+ _mapping: LMDB # FIXME CPython implementation detail
+ __slots__ = ()
+
+ def __contains__(self, item: object) -> bool:
+ key: str
+ value: object
+ key, value = item # type: ignore
+ try:
+ v = self._mapping[key]
+ except (KeyError, AttributeError):
+ return False
+ else:
+ return v == value
+
+ def __iter__(self) -> Iterator[tuple[str, bytes]]:
+ cursor = self._mapping.db.begin().cursor()
+ return ((_decode_key(k), v) for k, v in cursor.iternext(keys=True, values=True))
+
+
+class LMDBValuesView(ValuesView[bytes]):
+ _mapping: LMDB # FIXME CPython implementation detail
+ __slots__ = ()
+
+ def __contains__(self, value: object) -> bool:
+ return any(value == v for v in self)
+
+ def __iter__(self) -> Iterator[bytes]:
+ cursor = self._mapping.db.begin().cursor()
+ return cursor.iternext(keys=False, values=True)

zict/sieve.py

@@ -2,7 +2,6 @@
 
 from collections import defaultdict
 from collections.abc import Callable, Iterable, Iterator, Mapping, MutableMapping
-from itertools import chain
 from typing import Generic, TypeVar
 
 from zict.common import KT, VT, ZictBase, close, flush
@@ -92,21 +91,11 @@ def _do_update(self, items: Iterable[tuple[KT, VT]]) -> None:
  for key, _ in mitems:
  self.key_to_mapping[key] = mapping
 
- # FIXME dictionary views https://github.com/dask/zict/issues/61
- def keys(self) -> Iterator[KT]: # type: ignore
- return chain.from_iterable(self.mappings.values())
-
- def values(self) -> Iterator[VT]: # type: ignore
- return chain.from_iterable(m.values() for m in self.mappings.values())
-
- def items(self) -> Iterator[tuple[KT, VT]]: # type: ignore
- return chain.from_iterable(m.items() for m in self.mappings.values())
-
  def __len__(self) -> int:
  return sum(map(len, self.mappings.values()))
 
  def __iter__(self) -> Iterator[KT]:
- return self.keys()
+ return iter(self.key_to_mapping)
 
  def __contains__(self, key: object) -> bool:
  return key in self.key_to_mapping

zict/tests/test_buffer.py

@@ -228,3 +228,17 @@ def test_set_noevict():
  assert a == {"y": 3, "x": 1}
  assert b == {"z": 6}
  assert f2s == s2f == []
+
+
+def test_evict_restore_during_iter():
+ """Test that __iter__ won't be disrupted if another thread evicts or restores a key"""
+ buff = Buffer({"x": 1, "y": 2}, {"z": 3}, n=5)
+ assert list(buff) == ["x", "y", "z"]
+ it = iter(buff)
+ assert next(it) == "x"
+ buff.fast.evict("x")
+ assert next(it) == "y"
+ assert buff["x"] == 1
+ assert next(it) == "z"
+ with pytest.raises(StopIteration):
+ next(it)

zict/tests/test_cache.py

@@ -4,6 +4,7 @@
 import pytest
 
 from zict import Cache, WeakValueMapping
+from zict.tests import utils_test
 
 
 def test_cache_get_set_del():
@@ -129,3 +130,12 @@ class C:
  b = "bbb"
  d["b"] = b
  assert "b" not in d
+
+
+def test_mapping():
+ """
+ Test mapping interface for Cache().
+ """
+ buff = Cache({}, {})
+ utils_test.check_mapping(buff)
+ utils_test.check_closing(buff)

zict/tests/test_lru.py

@@ -33,7 +33,7 @@ def test_simple():
  assert "y" not in lru
 
  lru["a"] = 5
- assert set(lru.keys()) == {"z", "a"}
+ assert set(lru) == {"z", "a"}
 
 
 def test_str():