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

Remove dead custom frozendict implementations #854

Merged
merged 1 commit into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes.d/853.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove private and unused frozendict fallback implementations `_FrozenDictByInheritance` and `_FrozenDictByWrapping`.
128 changes: 3 additions & 125 deletions qupulse/utils/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,12 @@
import numpy
import sympy
import gmpy2

try:
from frozendict import frozendict
except ImportError:
warnings.warn("The frozendict package is not installed. We currently also ship a fallback frozendict which "
"will be removed in a future release.", category=DeprecationWarning)
frozendict = None
from frozendict import frozendict

import qupulse.utils.numeric as qupulse_numeric

__all__ = ["MeasurementWindow", "ChannelID", "HashableNumpyArray", "TimeType", "time_from_float", "DocStringABCMeta",
"SingletonABCMeta", "SequenceProxy", "frozendict"]
"SingletonABCMeta", "SequenceProxy"]

MeasurementWindow = typing.Tuple[str, numbers.Real, numbers.Real]
ChannelID = typing.Union[str, int]
Expand Down Expand Up @@ -399,121 +393,7 @@ def has_type_interface(obj: typing.Any, type_obj: typing.Type) -> bool:
_T_co_hash = typing.TypeVar('_T_co_hash', bound=typing.Hashable, covariant=True) # Any type covariant containers.

FrozenMapping = typing.Mapping[_KT_hash, _T_co_hash]


class _FrozenDictByInheritance(dict):
"""This is non mutable, hashable dict. It violates the Liskov substitution principle but is faster than wrapping.
It is not used by default and may be removed in the future.
"""
def __setitem__(self, key, value):
raise TypeError('FrozenDict is immutable')

def __delitem__(self, key):
raise TypeError('FrozenDict is immutable')

def update(self, *args, **kwargs):
raise TypeError('FrozenDict is immutable')

def setdefault(self, *args, **kwargs):
raise TypeError('FrozenDict is immutable')

def clear(self):
raise TypeError('FrozenDict is immutable')

def pop(self, *args, **kwargs):
raise TypeError('FrozenDict is immutable')

def popitem(self, *args, **kwargs):
raise TypeError('FrozenDict is immutable')

def copy(self):
return self

def to_dict(self) -> typing.Dict[_KT_hash, _T_co_hash]:
return super().copy()

def __hash__(self):
# faster than functools.reduce(operator.xor, map(hash, self.items())) but takes more memory
# TODO: investigate caching
return hash(frozenset(self.items()))


class _FrozenDictByWrapping(FrozenMapping):
"""Immutable dict like type.

There are the following possibilities in pure python:
- subclass dict (violates the Liskov substitution principle)
- wrap dict (slow construction and method indirection)
- abuse MappingProxyType (hard to add hash and make mutation difficult)



Wrapper around builtin dict without the mutating methods.

Hot path methods in __slots__ are the bound methods of the dict object. The other methods are wrappers.

Why not subclass dict and overwrite mutating methods:
roughly the same speed for __slot__ methods (a bit slower than native dict)
dict subclass always implements MutableMapping which makes type annotations useless
caching the hash value is slightly slower for the subclass

Only downside: This wrapper class needs to implement __init__ and copy the __slot__ methods which is an overhead of
~10 i.e. 250ns for empty subclass init vs. 4µs for empty wrapper init
"""
# made concessions in code style due to performance
_HOT_PATH_METHODS = ('keys', 'items', 'values', 'get', '__getitem__')
_PRIVATE_ATTRIBUTES = ('_hash', '_dict')
__slots__ = _HOT_PATH_METHODS + _PRIVATE_ATTRIBUTES

def __new__(cls, *args, **kwds):
"""Overwriting __new__ saves a factor of two for initialization. This is the relevant line from
Generic.__new__"""
return object.__new__(cls)

def __init__(self, *args, **kwargs):
inner_dict = dict(*args, **kwargs)
self._dict = inner_dict # type: typing.Dict[_KT_hash, _T_co_hash]
self._hash = None

self.__getitem__ = inner_dict.__getitem__
self.keys = inner_dict.keys
self.items = inner_dict.items
self.values = inner_dict.values
self.get = inner_dict.get

def __contains__(self, item: _KT_hash) -> bool:
return item in self._dict

def __iter__(self) -> typing.Iterator[_KT_hash]:
return iter(self._dict)

def __len__(self) -> int:
return len(self._dict)

def __repr__(self):
return '%s(%r)' % (self.__class__.__name__, self._dict)

def __hash__(self) -> int:
# use the local variable h to minimize getattr calls to minimum and reduce caching overhead
h = self._hash
if h is None:
self._hash = h = functools.reduce(operator.xor, map(hash, self.items()), 0xABCD0)
return h

def __eq__(self, other: typing.Mapping):
return other == self._dict

def copy(self):
return self

def to_dict(self) -> typing.Dict[_KT_hash, _T_co_hash]:
return self._dict.copy()


if frozendict is None:
FrozenDict = _FrozenDictByWrapping
else:
FrozenDict = frozendict
FrozenDict = frozendict


class SequenceProxy(collections.abc.Sequence):
Expand Down Expand Up @@ -550,5 +430,3 @@ def __eq__(self, other):
and all(x == y for x, y in zip(self, other)))
else:
return NotImplemented


102 changes: 1 addition & 101 deletions tests/utils/types_tests.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import unittest
import inspect

import numpy as np

from qupulse.utils.types import (HashableNumpyArray, SequenceProxy, _FrozenDictByWrapping,
_FrozenDictByInheritance)
from qupulse.utils.types import (HashableNumpyArray, SequenceProxy,)


class HashableNumpyArrayTest(unittest.TestCase):
Expand Down Expand Up @@ -36,101 +34,3 @@ def test_sequence_proxy(self):

with self.assertRaises(TypeError):
p[1] = 7


class FrozenDictTests(unittest.TestCase):
FrozenDictType = _FrozenDictByWrapping

"""This class can test general non mutable mappings"""
def setUp(self) -> None:
self.d = {'a': 1, 'b': 2}
self.f = self.FrozenDictType(self.d)
self.prev_state = dict(self.f)

def tearDown(self) -> None:
self.assertEqual(self.prev_state, dict(self.f))

def test_init(self):
d = {'a': 1, 'b': 2}

f1 = self.FrozenDictType(d)
f2 = self.FrozenDictType(**d)
f3 = self.FrozenDictType(d.items())

self.assertEqual(d, f1)
self.assertEqual(d, f2)
self.assertEqual(d, f3)

self.assertEqual(d.keys(), f1.keys())
self.assertEqual(d.keys(), f2.keys())
self.assertEqual(d.keys(), f3.keys())

self.assertEqual(set(d.items()), set(f1.items()))
self.assertEqual(set(d.items()), set(f2.items()))
self.assertEqual(set(d.items()), set(f3.items()))

def test_mapping(self):
d = {'a': 1, 'b': 2}
f = self.FrozenDictType(d)

self.assertEqual(len(d), len(f))
self.assertIn('a', f)
self.assertIn('b', f)
self.assertNotIn('c', f)

self.assertEqual(1, f['a'])
self.assertEqual(2, f['b'])

with self.assertRaisesRegex(KeyError, 'c'):
_ = f['c']

with self.assertRaises(TypeError):
f['a'] = 9

with self.assertRaises(TypeError):
del f['a']

def test_copy(self):
d = {'a': 1, 'b': 2}
f = self.FrozenDictType(d)
self.assertIs(f, f.copy())

def test_eq_and_hash(self):
d = {'a': 1, 'b': 2}

f1 = self.FrozenDictType(d)
f2 = self.FrozenDictType({'a': 1, 'b': 2})
f3 = self.FrozenDictType({'a': 1, 'c': 3})

self.assertEqual(f1, f2)
self.assertEqual(hash(f1), hash(f2))

self.assertNotEqual(f1, f3)


class FrozenDictByInheritanceTests(FrozenDictTests):
FrozenDictType = _FrozenDictByInheritance

def test_update(self):
with self.assertRaisesRegex(TypeError, 'immutable'):
self.f.update(d=5)

def test_setdefault(self):
with self.assertRaisesRegex(TypeError, 'immutable'):
self.f.setdefault('c', 3)
with self.assertRaisesRegex(TypeError, 'immutable'):
self.f.setdefault('a', 2)

def test_clear(self):
with self.assertRaisesRegex(TypeError, 'immutable'):
self.f.clear()

def test_pop(self):
with self.assertRaisesRegex(TypeError, 'immutable'):
self.f.pop()
with self.assertRaisesRegex(TypeError, 'immutable'):
self.f.pop('a')

def test_popitem(self):
with self.assertRaisesRegex(TypeError, 'immutable'):
self.f.popitem()
Loading