diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9214aeeda..3afc24a1e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,13 +9,13 @@ repos: types: [] - repo: https://gitlab.com/pycqa/flake8 - rev: 3.7.9 + rev: 3.8.0a2 hooks: - id: flake8 language_version: python3.8 - repo: https://github.com/asottile/seed-isort-config - rev: v1.9.4 + rev: v2.1.1 hooks: - id: seed-isort-config @@ -26,7 +26,7 @@ repos: additional_dependencies: [toml] - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v2.4.0 + rev: v2.5.0 hooks: - id: trailing-whitespace - id: end-of-file-fixer diff --git a/changelog.d/512.change.rst b/changelog.d/512.change.rst new file mode 100644 index 000000000..6c2e8f146 --- /dev/null +++ b/changelog.d/512.change.rst @@ -0,0 +1,6 @@ +It is now possible to prevent ``attrs`` from auto-generating the ``__setstate__`` and ``__getstate__`` methods that are required for pickling of slotted classes. + +Either pass ``@attr.s(getstate_setstate=False)`` or pass ``@attr.s(auto_detect=True)`` and implement them yourself: +if ``attrs`` finds either of the two methods directly on the decorated class, it assumes implicitly ``getstate_setstate=False`` (and implements neither). + +This option works with dict class but should never be necessary. diff --git a/changelog.d/513.change.rst b/changelog.d/513.change.rst new file mode 100644 index 000000000..6c2e8f146 --- /dev/null +++ b/changelog.d/513.change.rst @@ -0,0 +1,6 @@ +It is now possible to prevent ``attrs`` from auto-generating the ``__setstate__`` and ``__getstate__`` methods that are required for pickling of slotted classes. + +Either pass ``@attr.s(getstate_setstate=False)`` or pass ``@attr.s(auto_detect=True)`` and implement them yourself: +if ``attrs`` finds either of the two methods directly on the decorated class, it assumes implicitly ``getstate_setstate=False`` (and implements neither). + +This option works with dict class but should never be necessary. diff --git a/changelog.d/642.change.rst b/changelog.d/642.change.rst new file mode 100644 index 000000000..6c2e8f146 --- /dev/null +++ b/changelog.d/642.change.rst @@ -0,0 +1,6 @@ +It is now possible to prevent ``attrs`` from auto-generating the ``__setstate__`` and ``__getstate__`` methods that are required for pickling of slotted classes. + +Either pass ``@attr.s(getstate_setstate=False)`` or pass ``@attr.s(auto_detect=True)`` and implement them yourself: +if ``attrs`` finds either of the two methods directly on the decorated class, it assumes implicitly ``getstate_setstate=False`` (and implements neither). + +This option works with dict class but should never be necessary. diff --git a/docs/api.rst b/docs/api.rst index 0ddbc4ad3..ae4d1dfb9 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -16,7 +16,7 @@ What follows is the API explanation, if you'd like a more hands-on introduction, Core ---- -.. autofunction:: attr.s(these=None, repr_ns=None, repr=None, cmp=None, hash=None, init=None, slots=False, frozen=False, weakref_slot=True, str=False, auto_attribs=False, kw_only=False, cache_hash=False, auto_exc=False, eq=None, order=None, auto_detect=False, collect_by_mro=False) +.. autofunction:: attr.s(these=None, repr_ns=None, repr=None, cmp=None, hash=None, init=None, slots=False, frozen=False, weakref_slot=True, str=False, auto_attribs=False, kw_only=False, cache_hash=False, auto_exc=False, eq=None, order=None, auto_detect=False, collect_by_mro=False, getstate_setstate=None) .. note:: diff --git a/docs/glossary.rst b/docs/glossary.rst index 6edf9d102..a355d98bf 100644 --- a/docs/glossary.rst +++ b/docs/glossary.rst @@ -15,7 +15,7 @@ Glossary Their main advantage is that they use less memory on CPython [#pypy]_. - However they also come with a bunch of possibly surprising gotchas: + However they also come with several possibly surprising gotchas: - Slotted classes don't allow for any other attribute to be set except for those defined in one of the class' hierarchies ``__slots__``: @@ -41,14 +41,16 @@ Glossary .. note:: - If the ``@attr.s(slots=True)`` decorated class already implements the :meth:`__getstate__ ` and :meth:`__setstate__ ` methods, they will be *overridden* by ``attrs`` autogenerated implementation. + If the ``@attr.s(slots=True)`` decorated class already implements the :meth:`__getstate__ ` and :meth:`__setstate__ ` methods, they will be *overwritten* by ``attrs`` autogenerated implementation by default. + + This can be avoided by setting ``@attr.s(getstate_setstate=False)`` or by setting ``@attr.s(auto_detect=True)``. Also, `think twice `_ before using `pickle`. - Slotted classes are weak-referenceable by default. This can be disabled in CPython by passing ``weakref_slot=False`` to ``@attr.s`` [#pypyweakref]_. - - Since it's currently impossible to make a class slotted after it's created, ``attrs`` has to replace your class with a new one. + - Since it's currently impossible to make a class slotted after it's been created, ``attrs`` has to replace your class with a new one. While it tries to do that as graciously as possible, certain metaclass features like ``__init_subclass__`` do not work with slotted classes. diff --git a/src/attr/__init__.pyi b/src/attr/__init__.pyi index 19c5a81c2..ba8d7b3ad 100644 --- a/src/attr/__init__.pyi +++ b/src/attr/__init__.pyi @@ -188,6 +188,7 @@ def attrs( eq: Optional[bool] = ..., order: Optional[bool] = ..., auto_detect: bool = ..., + getstate_setstate: Optional[bool] = ..., ) -> _C: ... @overload def attrs( @@ -209,6 +210,7 @@ def attrs( eq: Optional[bool] = ..., order: Optional[bool] = ..., auto_detect: bool = ..., + getstate_setstate: Optional[bool] = ..., ) -> Callable[[_C], _C]: ... # TODO: add support for returning NamedTuple from the mypy plugin diff --git a/src/attr/_make.py b/src/attr/_make.py index 995555bd9..6c039abc3 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -546,6 +546,7 @@ def __init__( slots, frozen, weakref_slot, + getstate_setstate, auto_attribs, kw_only, cache_hash, @@ -576,6 +577,12 @@ def __init__( self._cls_dict["__setattr__"] = _frozen_setattrs self._cls_dict["__delattr__"] = _frozen_delattrs + if getstate_setstate: + ( + self._cls_dict["__getstate__"], + self._cls_dict["__setstate__"], + ) = self._make_getstate_setstate() + def __repr__(self): return "<_ClassBuilder(cls={cls})>".format(cls=self._cls.__name__) @@ -657,37 +664,6 @@ def _create_slots_class(self): if qualname is not None: cd["__qualname__"] = qualname - # __weakref__ is not writable. - state_attr_names = tuple( - an for an in self._attr_names if an != "__weakref__" - ) - - def slots_getstate(self): - """ - Automatically created by attrs. - """ - return tuple(getattr(self, name) for name in state_attr_names) - - hash_caching_enabled = self._cache_hash - - def slots_setstate(self, state): - """ - Automatically created by attrs. - """ - __bound_setattr = _obj_setattr.__get__(self, Attribute) - for name, value in zip(state_attr_names, state): - __bound_setattr(name, value) - - # The hash code cache is not included when the object is - # serialized, but it still needs to be initialized to None to - # indicate that the first call to __hash__ should be a cache miss. - if hash_caching_enabled: - __bound_setattr(_hash_cache_field, None) - - # slots and frozen require __getstate__/__setstate__ to work - cd["__getstate__"] = slots_getstate - cd["__setstate__"] = slots_setstate - # Create new class based on old class and our methods. cls = type(self._cls)(self._cls.__name__, self._cls.__bases__, cd) @@ -737,6 +713,40 @@ def __str__(self): self._cls_dict["__str__"] = self._add_method_dunders(__str__) return self + def _make_getstate_setstate(self): + """ + Create custom __setstate__ and __getstate__ methods. + """ + # __weakref__ is not writable. + state_attr_names = tuple( + an for an in self._attr_names if an != "__weakref__" + ) + + def slots_getstate(self): + """ + Automatically created by attrs. + """ + return tuple(getattr(self, name) for name in state_attr_names) + + hash_caching_enabled = self._cache_hash + + def slots_setstate(self, state): + """ + Automatically created by attrs. + """ + __bound_setattr = _obj_setattr.__get__(self, Attribute) + for name, value in zip(state_attr_names, state): + __bound_setattr(name, value) + + # The hash code cache is not included when the object is + # serialized, but it still needs to be initialized to None to + # indicate that the first call to __hash__ should be a cache + # miss. + if hash_caching_enabled: + __bound_setattr(_hash_cache_field, None) + + return slots_getstate, slots_setstate + def make_unhashable(self): self._cls_dict["__hash__"] = None return self @@ -849,7 +859,9 @@ def _determine_eq_order(cmp, eq, order, default_eq): return eq, order -def _determine_whether_to_implement(cls, flag, auto_detect, dunders): +def _determine_whether_to_implement( + cls, flag, auto_detect, dunders, default=True +): """ Check whether we should implement a set of methods for *cls*. @@ -857,20 +869,22 @@ def _determine_whether_to_implement(cls, flag, auto_detect, dunders): same as passed into @attr.s and *dunders* is a tuple of attribute names whose presence signal that the user has implemented it themselves. + Return *default* if no reason for either for or against is found. + auto_detect must be False on Python 2. """ - if flag is True or flag is None and auto_detect is False: - return True + if flag is True or flag is False: + return flag - if flag is False: - return False + if flag is None and auto_detect is False: + return default # Logically, flag is None and auto_detect is True here. for dunder in dunders: if _has_own_attribute(cls, dunder): return False - return True + return default def attrs( @@ -893,6 +907,7 @@ def attrs( order=None, auto_detect=False, collect_by_mro=False, + getstate_setstate=None, ): r""" A class decorator that adds `dunder @@ -1060,6 +1075,21 @@ def attrs( See issue `#428 `_ for more details. + :param Optional[bool] getstate_setstate: + .. note:: + This is usually only interesting for slotted classes and you should + probably just set *auto_detect* to `True`. + + If `True`, ``__getstate__`` and + ``__setstate__`` are generated and attached to the class. This is + necessary for slotted classes to be pickleable. If left `None`, it's + `True` by default for slotted classes and ``False`` for dict classes. + + If *auto_detect* is `True`, and *getstate_setstate* is left `None`, + and **either** ``__getstate__`` or ``__setstate__`` is detected directly + on the class (i.e. not inherited), it is set to `False` (this is usually + what you want). + .. versionadded:: 16.0.0 *slots* .. versionadded:: 16.1.0 *frozen* .. versionadded:: 16.3.0 *str* @@ -1086,6 +1116,7 @@ def attrs( .. versionadded:: 19.2.0 *eq* and *order* .. versionadded:: 20.1.0 *auto_detect* .. versionadded:: 20.1.0 *collect_by_mro* + .. versionadded:: 20.1.0 *getstate_setstate* """ if auto_detect and PY2: raise PythonTooOldError( @@ -1093,7 +1124,7 @@ def attrs( ) eq_, order_ = _determine_eq_order(cmp, eq, order, None) - hash_ = hash # workaround the lack of nonlocal + hash_ = hash # work around the lack of nonlocal def wrap(cls): @@ -1108,6 +1139,13 @@ def wrap(cls): slots, frozen, weakref_slot, + _determine_whether_to_implement( + cls, + getstate_setstate, + auto_detect, + ("__getstate__", "__setstate__"), + default=slots, + ), auto_attribs, kw_only, cache_hash, diff --git a/tests/test_make.py b/tests/test_make.py index 310faf0ea..8ccf191ee 100644 --- a/tests/test_make.py +++ b/tests/test_make.py @@ -1425,7 +1425,7 @@ class C(object): pass b = _ClassBuilder( - C, None, True, True, False, False, False, False, False, True + C, None, True, True, False, False, False, False, False, False, True ) assert "<_ClassBuilder(cls=C)>" == repr(b) @@ -1439,7 +1439,7 @@ class C(object): x = attr.ib() b = _ClassBuilder( - C, None, True, True, False, False, False, False, False, True + C, None, True, True, False, False, False, False, False, False, True ) cls = ( @@ -1510,6 +1510,7 @@ class C(object): slots=False, frozen=False, weakref_slot=True, + getstate_setstate=False, auto_attribs=False, is_exc=False, kw_only=False, @@ -2101,3 +2102,32 @@ def __le__(self, o): assert c1 == c1 assert c1.own_eq_called + + @pytest.mark.parametrize("slots", [True, False]) + def test_detects_setstate_getstate(self, slots): + """ + __getstate__ and __setstate__ are not overwritten if either is present. + """ + + @attr.s(slots=slots, auto_detect=True) + class C(object): + def __getstate__(self): + return ("hi",) + + assert None is getattr(C(), "__setstate__", None) + + @attr.s(slots=slots, auto_detect=True) + class C(object): + called = attr.ib(False) + + def __setstate__(self, state): + self.called = True + + i = C() + + assert False is i.called + + i.__setstate__(()) + + assert True is i.called + assert None is getattr(C(), "__getstate__", None) diff --git a/tests/test_slots.py b/tests/test_slots.py index 1d83297d5..b57fc639d 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -2,6 +2,7 @@ Unit tests for slots-related functionality. """ +import pickle import sys import types import weakref @@ -568,3 +569,59 @@ def f(self, a): super(C, self).__init__() C(field=1) + + +@attr.s(getstate_setstate=True) +class C2(object): + x = attr.ib() + + +@attr.s(slots=True, getstate_setstate=True) +class C2Slots(object): + x = attr.ib() + + +class TestPickle(object): + @pytest.mark.parametrize("protocol", range(pickle.HIGHEST_PROTOCOL)) + def test_pickleable_by_default(self, protocol): + """ + If nothing else is passed, slotted classes can be pickled and + unpickled with all supported protocols. + """ + i1 = C1Slots(1, 2) + i2 = pickle.loads(pickle.dumps(i1, protocol)) + + assert i1 == i2 + assert i1 is not i2 + + def test_no_getstate_setstate_for_dict_classes(self): + """ + As long as getstate_setstate is None, nothing is done to dict + classes. + """ + i = C1(1, 2) + + assert None is getattr(i, "__getstate__", None) + assert None is getattr(i, "__setstate__", None) + + def test_no_getstate_setstate_if_option_false(self): + """ + Don't add getstate/setstate if getstate_setstate is False. + """ + + @attr.s(slots=True, getstate_setstate=False) + class C(object): + x = attr.ib() + + i = C(42) + + assert None is getattr(i, "__getstate__", None) + assert None is getattr(i, "__setstate__", None) + + @pytest.mark.parametrize("cls", [C2(1), C2Slots(1)]) + def test_getstate_set_state_force_true(self, cls): + """ + If getstate_setstate is True, add them unconditionally. + """ + assert None is not getattr(cls, "__getstate__", None) + assert None is not getattr(cls, "__setstate__", None)