From 08fcfe91d4f44bc3273c00e5cc726bf961c950f2 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Sun, 22 Sep 2019 15:07:19 +0200 Subject: [PATCH] Split cmp into eq and order (#574) * Split cmp into eq and order Fixes #170 * Fix tests on old pypy3 versions Old as in: currently on AP. * Fix issue number and clarify newsfragment * Clarify behavior and interaction between cmp/eq/order * This sounds better * Address Julian's review comments * Missed a cmp * Test the behavior of Attribute.cmp * Make test more idiomatic * Explain assumptions * Clarify comment * Grammar * One more cmp! --- README.rst | 2 +- changelog.d/574.deprecation.rst | 11 ++ conftest.py | 17 --- docs/api.rst | 14 +- docs/examples.rst | 2 - docs/extending.rst | 14 +- docs/hashing.rst | 11 +- src/attr/__init__.pyi | 34 +++-- src/attr/_make.py | 218 ++++++++++++++++++++++++-------- tests/strategies.py | 6 +- tests/test_dark_magic.py | 102 +++++++++++++-- tests/test_dunders.py | 70 +++++----- tests/test_funcs.py | 15 +++ tests/test_make.py | 97 ++++++++++++-- tests/test_slots.py | 10 +- tests/typing_example.py | 7 + tests/utils.py | 11 +- 17 files changed, 478 insertions(+), 163 deletions(-) create mode 100644 changelog.d/574.deprecation.rst diff --git a/README.rst b/README.rst index e0a57691..565cae41 100644 --- a/README.rst +++ b/README.rst @@ -72,7 +72,7 @@ After *declaring* your attributes ``attrs`` gives you: - a concise and explicit overview of the class's attributes, - a nice human-readable ``__repr__``, -- a complete set of comparison methods, +- a complete set of comparison methods (equality and ordering), - an initializer, - and much more, diff --git a/changelog.d/574.deprecation.rst b/changelog.d/574.deprecation.rst new file mode 100644 index 00000000..130fe4d9 --- /dev/null +++ b/changelog.d/574.deprecation.rst @@ -0,0 +1,11 @@ +The ``cmp`` argument to ``attr.s()`` and ``attr.ib()`` is now deprecated. + +Please use ``eq`` to add equality methods (``__eq__`` and ``__ne__``) and ``order`` to add ordering methods (``__lt__``, ``__le__``, ``__gt__``, and ``__ge__``) instead – just like with `dataclasses `_. + +Both are effectively ``True`` by default but it's enough to set ``eq=False`` to disable both at once. +Passing ``eq=False, order=True`` explicitly will raise a ``ValueError`` though. + +Since this is arguably a deeper backward-compatibility break, it will have an extended deprecation period until 2021-06-01. +After that day, the ``cmp`` argument will be removed. + +``attr.Attribute`` also isn't orderable anymore. diff --git a/conftest.py b/conftest.py index 9f84c345..b83f69a5 100644 --- a/conftest.py +++ b/conftest.py @@ -2,8 +2,6 @@ from __future__ import absolute_import, division, print_function import sys -import pytest - from hypothesis import HealthCheck, settings @@ -15,21 +13,6 @@ def pytest_configure(config): settings.load_profile("patience") -@pytest.fixture(scope="session") -def C(): - """ - Return a simple but fully featured attrs class with an x and a y attribute. - """ - import attr - - @attr.s - class C(object): - x = attr.ib() - y = attr.ib() - - return C - - collect_ignore = [] if sys.version_info[:2] < (3, 6): collect_ignore.extend( diff --git a/docs/api.rst b/docs/api.rst index 76deae36..f68aaa07 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=True, cmp=True, hash=None, init=True, slots=False, frozen=False, weakref_slot=True, str=False, auto_attribs=False, kw_only=False, cache_hash=False, auto_exc=False) +.. autofunction:: attr.s(these=None, repr_ns=None, repr=True, cmp=None, hash=None, init=True, 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) .. note:: @@ -102,7 +102,7 @@ Core ... class C(object): ... x = attr.ib() >>> attr.fields(C).x - Attribute(name='x', default=NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False) + Attribute(name='x', default=NOTHING, validator=None, repr=True, eq=True, order=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False) .. autofunction:: attr.make_class @@ -174,9 +174,9 @@ Helpers ... x = attr.ib() ... y = attr.ib() >>> attr.fields(C) - (Attribute(name='x', default=NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False), Attribute(name='y', default=NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False)) + (Attribute(name='x', default=NOTHING, validator=None, repr=True, eq=True, order=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False), Attribute(name='y', default=NOTHING, validator=None, repr=True, eq=True, order=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False)) >>> attr.fields(C)[1] - Attribute(name='y', default=NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False) + Attribute(name='y', default=NOTHING, validator=None, repr=True, eq=True, order=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False) >>> attr.fields(C).y is attr.fields(C)[1] True @@ -191,9 +191,9 @@ Helpers ... x = attr.ib() ... y = attr.ib() >>> attr.fields_dict(C) - {'x': Attribute(name='x', default=NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False), 'y': Attribute(name='y', default=NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False)} + {'x': Attribute(name='x', default=NOTHING, validator=None, repr=True, eq=True, order=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False), 'y': Attribute(name='y', default=NOTHING, validator=None, repr=True, eq=True, order=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False)} >>> attr.fields_dict(C)['y'] - Attribute(name='y', default=NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False) + Attribute(name='y', default=NOTHING, validator=None, repr=True, eq=True, order=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False) >>> attr.fields_dict(C)['y'] is attr.fields(C).y True @@ -288,7 +288,7 @@ See :func:`asdict` for examples. >>> attr.validate(i) Traceback (most recent call last): ... - TypeError: ("'x' must be (got '1' that is a ).", Attribute(name='x', default=NOTHING, validator=>, repr=True, cmp=True, hash=None, init=True, type=None, kw_only=False), , '1') + TypeError: ("'x' must be (got '1' that is a ).", ...) Validators can be globally disabled if you want to run them only in development and tests but not in production because you fear their performance impact: diff --git a/docs/examples.rst b/docs/examples.rst index c47099cb..ae40777e 100644 --- a/docs/examples.rst +++ b/docs/examples.rst @@ -534,8 +534,6 @@ The generated ``__init__`` method will have an attribute called ``__annotations_ However it's useful for writing your own validators or serialization frameworks. -.. _slots: - Slots ----- diff --git a/docs/extending.rst b/docs/extending.rst index dcaebee6..4ee201ed 100644 --- a/docs/extending.rst +++ b/docs/extending.rst @@ -16,7 +16,7 @@ So it is fairly simple to build your own decorators on top of ``attrs``: ... @attr.s ... class C(object): ... a = attr.ib() - (Attribute(name='a', default=NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False),) + (Attribute(name='a', default=NOTHING, validator=None, repr=True, eq=True, order=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False),) .. warning:: @@ -99,10 +99,18 @@ Here are some tips for effective use of metadata: >>> MY_TYPE_METADATA = '__my_type_metadata' >>> - >>> def typed(cls, default=attr.NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, metadata={}, type=None, converter=None): + >>> def typed( + ... cls, default=attr.NOTHING, validator=None, repr=True, + ... eq=True, order=None, hash=None, init=True, metadata={}, + ... type=None, converter=None + ... ): ... metadata = dict() if not metadata else metadata ... metadata[MY_TYPE_METADATA] = cls - ... return attr.ib(default, validator, repr, cmp, hash, init, metadata, type, converter) + ... return attr.ib( + ... default=default, validator=validator, repr=repr, + ... eq=eq, order=order, hash=hash, init=init, + ... metadata=metadata, type=type, converter=converter + ... ) >>> >>> @attr.s ... class C(object): diff --git a/docs/hashing.rst b/docs/hashing.rst index 8bf35402..30888f97 100644 --- a/docs/hashing.rst +++ b/docs/hashing.rst @@ -10,7 +10,7 @@ Hash Method Generation Leave it at ``None`` which means that ``attrs`` will do the right thing for you, depending on the other parameters: - If you want to make objects hashable by value: use ``@attr.s(frozen=True)``. - - If you want hashing and comparison by object identity: use ``@attr.s(cmp=False)`` + - If you want hashing and equality by object identity: use ``@attr.s(eq=False)`` Setting ``hash`` yourself can have unexpected consequences so we recommend to tinker with it only if you know exactly what you're doing. @@ -34,13 +34,13 @@ Because according to the definition_ from the official Python docs, the returned Python 2 on the other hand will happily let you shoot your foot off. Unfortunately ``attrs`` currently mimics Python 2's behavior for backward compatibility reasons if you set ``hash=False``. - The *correct way* to achieve hashing by id is to set ``@attr.s(cmp=False)``. - Setting ``@attr.s(hash=False)`` (that implies ``cmp=True``) is almost certainly a *bug*. + The *correct way* to achieve hashing by id is to set ``@attr.s(eq=False)``. + Setting ``@attr.s(hash=False)`` (which implies ``eq=True``) is almost certainly a *bug*. .. warning:: Be careful when subclassing! - Setting ``cmp=False`` on class whose base class has a non-default ``__hash__`` method, will *not* make ``attrs`` remove that ``__hash__`` for you. + Setting ``eq=False`` on a class whose base class has a non-default ``__hash__`` method will *not* make ``attrs`` remove that ``__hash__`` for you. It is part of ``attrs``'s philosophy to only *add* to classes so you have the freedom to customize your classes as you wish. So if you want to *get rid* of methods, you'll have to do it by hand. @@ -65,8 +65,9 @@ For a more thorough explanation of this topic, please refer to this blog post: ` Hashing and Mutability ---------------------- + Changing any field involved in hash code computation after the first call to ``__hash__`` (typically this would be after its insertion into a hash-based collection) can result in silent bugs. -Therefore, it is strongly recommended that hashable classes be ``frozen.`` +Therefore, it is strongly recommended that hashable classes be ``frozen``. Beware, however, that this is not a complete guarantee of safety: if a field points to an object and that object is mutated, the hash code may change, but ``frozen`` will not protect you. diff --git a/src/attr/__init__.pyi b/src/attr/__init__.pyi index e331ea62..03bcf89f 100644 --- a/src/attr/__init__.pyi +++ b/src/attr/__init__.pyi @@ -53,16 +53,14 @@ class Attribute(Generic[_T]): validator: Optional[_ValidatorType[_T]] repr: _ReprArgType cmp: bool + eq: bool + order: bool hash: Optional[bool] init: bool converter: Optional[_ConverterType[_T]] metadata: Dict[Any, Any] type: Optional[Type[_T]] kw_only: bool - def __lt__(self, x: Attribute[_T]) -> bool: ... - def __le__(self, x: Attribute[_T]) -> bool: ... - def __gt__(self, x: Attribute[_T]) -> bool: ... - def __ge__(self, x: Attribute[_T]) -> bool: ... # NOTE: We had several choices for the annotation to use for type arg: # 1) Type[_T] @@ -92,7 +90,7 @@ def attrib( default: None = ..., validator: None = ..., repr: _ReprArgType = ..., - cmp: bool = ..., + cmp: Optional[bool] = ..., hash: Optional[bool] = ..., init: bool = ..., metadata: Optional[Mapping[Any, Any]] = ..., @@ -100,6 +98,8 @@ def attrib( converter: None = ..., factory: None = ..., kw_only: bool = ..., + eq: Optional[bool] = ..., + order: Optional[bool] = ..., ) -> Any: ... # This form catches an explicit None or no default and infers the type from the other arguments. @@ -108,7 +108,7 @@ def attrib( default: None = ..., validator: Optional[_ValidatorArgType[_T]] = ..., repr: _ReprArgType = ..., - cmp: bool = ..., + cmp: Optional[bool] = ..., hash: Optional[bool] = ..., init: bool = ..., metadata: Optional[Mapping[Any, Any]] = ..., @@ -116,6 +116,8 @@ def attrib( converter: Optional[_ConverterType[_T]] = ..., factory: Optional[Callable[[], _T]] = ..., kw_only: bool = ..., + eq: Optional[bool] = ..., + order: Optional[bool] = ..., ) -> _T: ... # This form catches an explicit default argument. @@ -124,7 +126,7 @@ def attrib( default: _T, validator: Optional[_ValidatorArgType[_T]] = ..., repr: _ReprArgType = ..., - cmp: bool = ..., + cmp: Optional[bool] = ..., hash: Optional[bool] = ..., init: bool = ..., metadata: Optional[Mapping[Any, Any]] = ..., @@ -132,6 +134,8 @@ def attrib( converter: Optional[_ConverterType[_T]] = ..., factory: Optional[Callable[[], _T]] = ..., kw_only: bool = ..., + eq: Optional[bool] = ..., + order: Optional[bool] = ..., ) -> _T: ... # This form covers type=non-Type: e.g. forward references (str), Any @@ -140,7 +144,7 @@ def attrib( default: Optional[_T] = ..., validator: Optional[_ValidatorArgType[_T]] = ..., repr: _ReprArgType = ..., - cmp: bool = ..., + cmp: Optional[bool] = ..., hash: Optional[bool] = ..., init: bool = ..., metadata: Optional[Mapping[Any, Any]] = ..., @@ -148,6 +152,8 @@ def attrib( converter: Optional[_ConverterType[_T]] = ..., factory: Optional[Callable[[], _T]] = ..., kw_only: bool = ..., + eq: Optional[bool] = ..., + order: Optional[bool] = ..., ) -> Any: ... @overload def attrs( @@ -155,7 +161,7 @@ def attrs( these: Optional[Dict[str, Any]] = ..., repr_ns: Optional[str] = ..., repr: bool = ..., - cmp: bool = ..., + cmp: Optional[bool] = ..., hash: Optional[bool] = ..., init: bool = ..., slots: bool = ..., @@ -166,6 +172,8 @@ def attrs( kw_only: bool = ..., cache_hash: bool = ..., auto_exc: bool = ..., + eq: Optional[bool] = ..., + order: Optional[bool] = ..., ) -> _C: ... @overload def attrs( @@ -173,7 +181,7 @@ def attrs( these: Optional[Dict[str, Any]] = ..., repr_ns: Optional[str] = ..., repr: bool = ..., - cmp: bool = ..., + cmp: Optional[bool] = ..., hash: Optional[bool] = ..., init: bool = ..., slots: bool = ..., @@ -184,6 +192,8 @@ def attrs( kw_only: bool = ..., cache_hash: bool = ..., auto_exc: bool = ..., + eq: Optional[bool] = ..., + order: Optional[bool] = ..., ) -> Callable[[_C], _C]: ... # TODO: add support for returning NamedTuple from the mypy plugin @@ -202,7 +212,7 @@ def make_class( bases: Tuple[type, ...] = ..., repr_ns: Optional[str] = ..., repr: bool = ..., - cmp: bool = ..., + cmp: Optional[bool] = ..., hash: Optional[bool] = ..., init: bool = ..., slots: bool = ..., @@ -213,6 +223,8 @@ def make_class( kw_only: bool = ..., cache_hash: bool = ..., auto_exc: bool = ..., + eq: Optional[bool] = ..., + order: Optional[bool] = ..., ) -> type: ... # _funcs -- diff --git a/src/attr/_make.py b/src/attr/_make.py index 51f67f7b..02f6e23c 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -5,6 +5,7 @@ import linecache import sys import threading import uuid +import warnings from operator import itemgetter @@ -73,7 +74,7 @@ def attrib( default=NOTHING, validator=None, repr=True, - cmp=True, + cmp=None, hash=None, init=True, metadata=None, @@ -81,6 +82,8 @@ def attrib( converter=None, factory=None, kw_only=False, + eq=None, + order=None, ): """ Create a new attribute on a class. @@ -135,10 +138,15 @@ def attrib( as-is, i.e. it will be used directly *instead* of calling ``repr()`` (the default). :type repr: a ``bool`` or a ``callable`` to use a custom function. - :param bool cmp: Include this attribute in the generated comparison methods - (``__eq__`` et al). + :param bool eq: If ``True`` (default), include this attribute in the + generated ``__eq__`` and ``__ne__`` methods that check two instances + for equality. + :param bool order: If ``True`` (default), include this attributes in the + generated ``__lt__``, ``__le__``, ``__gt__`` and ``__ge__`` methods. + :param bool cmp: Setting to ``True`` is equivalent to setting ``eq=True, + order=True``. Deprecated in favor of *eq* and *order*. :param hash: Include this attribute in the generated ``__hash__`` - method. If ``None`` (default), mirror *cmp*'s value. This is the + method. If ``None`` (default), mirror *eq*'s value. This is the correct behavior according the Python spec. Setting this value to anything else than ``None`` is *discouraged*. :type hash: ``bool`` or ``None`` @@ -171,7 +179,7 @@ def attrib( .. versionadded:: 16.3.0 *metadata* .. versionchanged:: 17.1.0 *validator* can be a ``list`` now. .. versionchanged:: 17.1.0 - *hash* is ``None`` and therefore mirrors *cmp* by default. + *hash* is ``None`` and therefore mirrors *eq* by default. .. versionadded:: 17.3.0 *type* .. deprecated:: 17.4.0 *convert* .. versionadded:: 17.4.0 *converter* as a replacement for the deprecated @@ -181,7 +189,11 @@ def attrib( .. versionadded:: 18.2.0 *kw_only* .. versionchanged:: 19.2.0 *convert* keyword argument removed .. versionchanged:: 19.2.0 *repr* also accepts a custom callable. + .. deprecated:: 19.2.0 *cmp* Removal on or after 2021-06-01. + .. versionadded:: 19.2.0 *eq* and *order* """ + eq, order = _determine_eq_order(cmp, eq, order) + if hash is not None and hash is not True and hash is not False: raise TypeError( "Invalid value for hash. Must be True, False, or None." @@ -204,13 +216,15 @@ def attrib( default=default, validator=validator, repr=repr, - cmp=cmp, + cmp=None, hash=hash, init=init, converter=converter, metadata=metadata, type=type, kw_only=kw_only, + eq=eq, + order=order, ) @@ -678,14 +692,22 @@ class _ClassBuilder(object): return self - def add_cmp(self): + def add_eq(self): cd = self._cls_dict - cd["__eq__"], cd["__ne__"], cd["__lt__"], cd["__le__"], cd[ - "__gt__" - ], cd["__ge__"] = ( + cd["__eq__"], cd["__ne__"] = ( self._add_method_dunders(meth) - for meth in _make_cmp(self._cls, self._attrs) + for meth in _make_eq(self._cls, self._attrs) + ) + + return self + + def add_order(self): + cd = self._cls_dict + + cd["__lt__"], cd["__le__"], cd["__gt__"], cd["__ge__"] = ( + self._add_method_dunders(meth) + for meth in _make_order(self._cls, self._attrs) ) return self @@ -709,12 +731,45 @@ class _ClassBuilder(object): return method +_CMP_DEPRECATION = ( + "The usage of `cmp` is deprecated and will be removed on or after " + "2021-06-01. Please use `eq` and `order` instead." +) + + +def _determine_eq_order(cmp, eq, order): + """ + Validate the combination of *cmp*, *eq*, and *order*. Derive the effective + values of eq and order. + """ + if cmp is not None and any((eq is not None, order is not None)): + raise ValueError("Don't mix `cmp` with `eq' and `order`.") + + # cmp takes precedence due to bw-compatibility. + if cmp is not None: + warnings.warn(_CMP_DEPRECATION, DeprecationWarning, stacklevel=3) + + return cmp, cmp + + # If left None, equality is on and ordering mirrors equality. + if eq is None: + eq = True + + if order is None: + order = eq + + if eq is False and order is True: + raise ValueError("`order` can only be True if `eq` is True too.") + + return eq, order + + def attrs( maybe_cls=None, these=None, repr_ns=None, repr=True, - cmp=True, + cmp=None, hash=None, init=True, slots=False, @@ -725,6 +780,8 @@ def attrs( kw_only=False, cache_hash=False, auto_exc=False, + eq=None, + order=None, ): r""" A class decorator that adds `dunder @@ -754,17 +811,28 @@ def attrs( :param bool str: Create a ``__str__`` method that is identical to ``__repr__``. This is usually not necessary except for `Exception`\ s. - :param bool cmp: Create ``__eq__``, ``__ne__``, ``__lt__``, ``__le__``, - ``__gt__``, and ``__ge__`` methods that compare the class as if it were - a tuple of its ``attrs`` attributes. But the attributes are *only* - compared, if the types of both classes are *identical*! + :param bool eq: If ``True`` or ``None`` (default), add ``__eq__`` and + ``__ne__`` methods that check two instances for equality. + + They compare the instances as if they were tuples of their ``attrs`` + attributes, but only iff the types of both classes are *identical*! + :type eq: `bool` or `None` + :param bool order: If ``True``, add ``__lt__``, ``__le__``, ``__gt__``, + and ``__ge__`` methods that behave like *eq* above and allow instances + to be ordered. If ``None`` (default) mirror value of *eq*. + :type order: `bool` or `None` + :param cmp: Setting to ``True`` is equivalent to setting ``eq=True, + order=True``. Deprecated in favor of *eq* and *order*, has precedence + over them for backward-compatibility though. Must not be mixed with + *eq* or *order*. + :type cmp: `bool` or `None` :param hash: If ``None`` (default), the ``__hash__`` method is generated - according how *cmp* and *frozen* are set. + according how *eq* and *frozen* are set. 1. If *both* are True, ``attrs`` will generate a ``__hash__`` for you. - 2. If *cmp* is True and *frozen* is False, ``__hash__`` will be set to + 2. If *eq* is True and *frozen* is False, ``__hash__`` will be set to None, marking it unhashable (which it is). - 3. If *cmp* is False, ``__hash__`` will be left untouched meaning the + 3. If *eq* is False, ``__hash__`` will be left untouched meaning the ``__hash__`` method of the base class will be used (if base class is ``object``, this means it will fall back to id-based hashing.). @@ -838,10 +906,10 @@ def attrs( following happens to behave like a well-behaved Python exceptions class: - - the values for *cmp* and *hash* are ignored and the instances compare - and hash by the instance's ids (N.B. ``attrs`` will *not* remove - existing implementations of ``__hash__`` or the equality methods. It - just won't add own ones.), + - the values for *eq*, *order*, and *hash* are ignored and the + instances compare and hash by the instance's ids (N.B. ``attrs`` will + *not* remove existing implementations of ``__hash__`` or the equality + methods. It just won't add own ones.), - all attributes that are either passed into ``__init__`` or have a default value are additionally available as a tuple in the ``args`` attribute, @@ -869,7 +937,10 @@ def attrs( .. versionadded:: 18.2.0 *kw_only* .. versionadded:: 18.2.0 *cache_hash* .. versionadded:: 19.1.0 *auto_exc* + .. deprecated:: 19.2.0 *cmp* Removal on or after 2021-06-01. + .. versionadded:: 19.2.0 *eq* and *order* """ + eq, order = _determine_eq_order(cmp, eq, order) def wrap(cls): @@ -894,15 +965,17 @@ def attrs( builder.add_repr(repr_ns) if str is True: builder.add_str() - if cmp is True and not is_exc: - builder.add_cmp() + if eq is True and not is_exc: + builder.add_eq() + if order is True and not is_exc: + builder.add_order() if hash is not True and hash is not False and hash is not None: # Can't use `hash in` because 1 == True for example. raise TypeError( "Invalid value for hash. Must be True, False, or None." ) - elif hash is False or (hash is None and cmp is False) or is_exc: + elif hash is False or (hash is None and eq is False) or is_exc: # Don't do anything. Should fall back to __object__'s __hash__ # which is by id. if cache_hash: @@ -911,7 +984,7 @@ def attrs( " hashing must be either explicitly or implicitly " "enabled." ) - elif hash is True or (hash is None and cmp is True and frozen is True): + elif hash is True or (hash is None and eq is True and frozen is True): # Build a __hash__ if told so, or if it's safe. builder.add_hash() else: @@ -1013,9 +1086,7 @@ def _generate_unique_filename(cls, func_name): def _make_hash(cls, attrs, frozen, cache_hash): attrs = tuple( - a - for a in attrs - if a.hash is True or (a.hash is None and a.cmp is True) + a for a in attrs if a.hash is True or (a.hash is None and a.eq is True) ) tab = " " @@ -1093,8 +1164,8 @@ def __ne__(self, other): return not result -def _make_cmp(cls, attrs): - attrs = [a for a in attrs if a.cmp] +def _make_eq(cls, attrs): + attrs = [a for a in attrs if a.eq] unique_filename = _generate_unique_filename(cls, "eq") lines = [ @@ -1129,8 +1200,11 @@ def _make_cmp(cls, attrs): script.splitlines(True), unique_filename, ) - eq = locs["__eq__"] - ne = __ne__ + return locs["__eq__"], __ne__ + + +def _make_order(cls, attrs): + attrs = [a for a in attrs if a.order] def attrs_to_tuple(obj): """ @@ -1174,19 +1248,17 @@ def _make_cmp(cls, attrs): return NotImplemented - return eq, ne, __lt__, __le__, __gt__, __ge__ + return __lt__, __le__, __gt__, __ge__ -def _add_cmp(cls, attrs=None): +def _add_eq(cls, attrs=None): """ - Add comparison methods to *cls*. + Add equality methods to *cls* with *attrs*. """ if attrs is None: attrs = cls.__attrs_attrs__ - cls.__eq__, cls.__ne__, cls.__lt__, cls.__le__, cls.__gt__, cls.__ge__ = _make_cmp( # noqa - cls, attrs - ) + cls.__eq__, cls.__ne__ = _make_eq(cls, attrs) return cls @@ -1682,7 +1754,8 @@ class Attribute(object): "default", "validator", "repr", - "cmp", + "eq", + "order", "hash", "init", "metadata", @@ -1697,14 +1770,18 @@ class Attribute(object): default, validator, repr, - cmp, + cmp, # XXX: unused, remove along with other cmp code. hash, init, metadata=None, type=None, converter=None, kw_only=False, + eq=None, + order=None, ): + eq, order = _determine_eq_order(cmp, eq, order) + # Cache this descriptor here to speed things up later. bound_setattr = _obj_setattr.__get__(self, Attribute) @@ -1714,7 +1791,8 @@ class Attribute(object): bound_setattr("default", default) bound_setattr("validator", validator) bound_setattr("repr", repr) - bound_setattr("cmp", cmp) + bound_setattr("eq", eq) + bound_setattr("order", order) bound_setattr("hash", hash) bound_setattr("init", init) bound_setattr("converter", converter) @@ -1757,9 +1835,19 @@ class Attribute(object): validator=ca._validator, default=ca._default, type=type, + cmp=None, **inst_dict ) + @property + def cmp(self): + """ + Simulate the presence of a cmp attribute and warn. + """ + warnings.warn(_CMP_DEPRECATION, DeprecationWarning, stacklevel=2) + + return self.eq and self.order + # Don't use attr.assoc since fields(Attribute) doesn't work def _assoc(self, **changes): """ @@ -1807,7 +1895,9 @@ _a = [ default=NOTHING, validator=None, repr=True, - cmp=True, + cmp=None, + eq=True, + order=False, hash=(name != "metadata"), init=True, ) @@ -1815,7 +1905,7 @@ _a = [ ] Attribute = _add_hash( - _add_cmp(_add_repr(Attribute, attrs=_a), attrs=_a), + _add_eq(_add_repr(Attribute, attrs=_a), attrs=_a), attrs=[a for a in _a if a.hash], ) @@ -1833,7 +1923,8 @@ class _CountingAttr(object): "counter", "_default", "repr", - "cmp", + "eq", + "order", "hash", "init", "metadata", @@ -1848,22 +1939,34 @@ class _CountingAttr(object): default=NOTHING, validator=None, repr=True, - cmp=True, + cmp=None, hash=True, init=True, kw_only=False, + eq=True, + order=False, + ) + for name in ( + "counter", + "_default", + "repr", + "eq", + "order", + "hash", + "init", ) - for name in ("counter", "_default", "repr", "cmp", "hash", "init") ) + ( Attribute( name="metadata", default=None, validator=None, repr=True, - cmp=True, + cmp=None, hash=False, init=True, kw_only=False, + eq=True, + order=False, ), ) cls_counter = 0 @@ -1873,13 +1976,15 @@ class _CountingAttr(object): default, validator, repr, - cmp, + cmp, # XXX: unused, remove along with cmp hash, init, converter, metadata, type, kw_only, + eq, + order, ): _CountingAttr.cls_counter += 1 self.counter = _CountingAttr.cls_counter @@ -1890,7 +1995,8 @@ class _CountingAttr(object): else: self._validator = validator self.repr = repr - self.cmp = cmp + self.eq = eq + self.order = order self.hash = hash self.init = init self.converter = converter @@ -1930,7 +2036,7 @@ class _CountingAttr(object): return meth -_CountingAttr = _add_cmp(_add_repr(_CountingAttr)) +_CountingAttr = _add_eq(_add_repr(_CountingAttr)) @attrs(slots=True, init=False, hash=True) @@ -2011,6 +2117,14 @@ def make_class(name, attrs, bases=(object,), **attributes_arguments): except (AttributeError, ValueError): pass + # We do it here for proper warnings with meaningful stacklevel. + cmp = attributes_arguments.pop("cmp", None) + attributes_arguments["eq"], attributes_arguments[ + "order" + ] = _determine_eq_order( + cmp, attributes_arguments.get("eq"), attributes_arguments.get("order") + ) + return _attrs(these=cls_dict, **attributes_arguments)(type_) diff --git a/tests/strategies.py b/tests/strategies.py index 1ebb9e4a..833778e7 100644 --- a/tests/strategies.py +++ b/tests/strategies.py @@ -14,6 +14,9 @@ import attr from .utils import make_class +optional_bool = st.one_of(st.none(), st.booleans()) + + def gen_attr_names(): """ Generate names for attributes, 'a'...'z', then 'aa'...'zz'. @@ -131,7 +134,8 @@ def simple_attrs_with_metadata(draw): default=c_attr._default, validator=c_attr._validator, repr=c_attr.repr, - cmp=c_attr.cmp, + eq=c_attr.eq, + order=c_attr.order, hash=c_attr.hash, init=c_attr.init, metadata=metadata, diff --git a/tests/test_dark_magic.py b/tests/test_dark_magic.py index 778fc9f3..c9b8b055 100644 --- a/tests/test_dark_magic.py +++ b/tests/test_dark_magic.py @@ -11,15 +11,17 @@ from copy import deepcopy import pytest import six -from hypothesis import given +from hypothesis import assume, given from hypothesis.strategies import booleans import attr -from attr._compat import TYPE +from attr._compat import PY2, TYPE from attr._make import NOTHING, Attribute from attr.exceptions import FrozenInstanceError +from .strategies import optional_bool + @attr.s class C1(object): @@ -124,7 +126,9 @@ class TestDarkMagic(object): default=foo, validator=None, repr=True, - cmp=True, + cmp=None, + eq=True, + order=True, hash=None, init=True, ), @@ -133,7 +137,9 @@ class TestDarkMagic(object): default=attr.Factory(list), validator=None, repr=True, - cmp=True, + cmp=None, + eq=True, + order=True, hash=None, init=True, ), @@ -181,13 +187,16 @@ class TestDarkMagic(object): `attr.make_class` works. """ PC = attr.make_class("PC", ["a", "b"], slots=slots, frozen=frozen) + assert ( Attribute( name="a", default=NOTHING, validator=None, repr=True, - cmp=True, + cmp=None, + eq=True, + order=True, hash=None, init=True, ), @@ -196,7 +205,9 @@ class TestDarkMagic(object): default=NOTHING, validator=None, repr=True, - cmp=True, + cmp=None, + eq=True, + order=True, hash=None, init=True, ), @@ -380,7 +391,7 @@ class TestDarkMagic(object): HashByIDBackwardCompat(1) ) - @attr.s(hash=False, cmp=False) + @attr.s(hash=False, eq=False) class HashByID(object): x = attr.ib() @@ -412,10 +423,10 @@ class TestDarkMagic(object): @pytest.mark.parametrize("slots", [True, False]) def test_hash_false_cmp_false(self, slots): """ - hash=False and cmp=False make a class hashable by ID. + hash=False and eq=False make a class hashable by ID. """ - @attr.s(hash=False, cmp=False, slots=slots) + @attr.s(hash=False, eq=False, slots=slots) class C(object): pass @@ -581,3 +592,76 @@ class TestDarkMagic(object): x = attr.ib() FooError(1) + + @pytest.mark.parametrize("slots", [True, False]) + @pytest.mark.parametrize("frozen", [True, False]) + def test_eq_only(self, slots, frozen): + """ + Classes with order=False cannot be ordered. + + Python 3 throws a TypeError, in Python2 we have to check for the + absence. + """ + + @attr.s(eq=True, order=False, slots=slots, frozen=frozen) + class C(object): + x = attr.ib() + + if not PY2: + possible_errors = ( + "unorderable types: C() < C()", + "'<' not supported between instances of 'C' and 'C'", + "unorderable types: C < C", # old PyPy 3 + ) + + with pytest.raises(TypeError) as ei: + C(5) < C(6) + + assert ei.value.args[0] in possible_errors + else: + i = C(42) + for m in ("lt", "le", "gt", "ge"): + assert None is getattr(i, "__%s__" % (m,), None) + + @given(cmp=optional_bool, eq=optional_bool, order=optional_bool) + def test_cmp_deprecated_attribute(self, cmp, eq, order): + """ + Accessing Attribute.cmp raises a deprecation warning but returns True + if cmp is True, or eq and order are *both* effectively True. + """ + # These cases are invalid and raise a ValueError. + assume(cmp is None or (eq is None and order is None)) + assume(not (eq is False and order is True)) + + if cmp is not None: + rv = cmp + elif eq is True or eq is None: + rv = order is None or order is True + elif cmp is None and eq is None and order is None: + rv = True + elif cmp is None or eq is None: + rv = False + else: + pytest.fail( + "Unexpected state: cmp=%r eq=%r order=%r" % (cmp, eq, order) + ) + + with pytest.deprecated_call() as dc: + + @attr.s + class C(object): + x = attr.ib(cmp=cmp, eq=eq, order=order) + + assert rv == attr.fields(C).x.cmp + + if cmp is not None: + # Remove warning from creating the attribute if cmp is not None. + dc.pop() + + w, = dc.list + + assert ( + "The usage of `cmp` is deprecated and will be removed on or after " + "2021-06-01. Please use `eq` and `order` instead." + == w.message.args[0] + ) diff --git a/tests/test_dunders.py b/tests/test_dunders.py index 84e092b4..d679ecff 100644 --- a/tests/test_dunders.py +++ b/tests/test_dunders.py @@ -29,8 +29,10 @@ from attr.validators import instance_of from .utils import simple_attr, simple_class -CmpC = simple_class(cmp=True) -CmpCSlots = simple_class(cmp=True, slots=True) +EqC = simple_class(eq=True) +EqCSlots = simple_class(eq=True, slots=True) +OrderC = simple_class(order=True) +OrderCSlots = simple_class(order=True, slots=True) ReprC = simple_class(repr=True) ReprCSlots = simple_class(repr=True, slots=True) @@ -38,10 +40,10 @@ ReprCSlots = simple_class(repr=True, slots=True) # implicitly. The "Cached" versions are the same, except with hash code # caching enabled HashC = simple_class(hash=True) -HashCSlots = simple_class(hash=None, cmp=True, frozen=True, slots=True) +HashCSlots = simple_class(hash=None, eq=True, frozen=True, slots=True) HashCCached = simple_class(hash=True, cache_hash=True) HashCSlotsCached = simple_class( - hash=None, cmp=True, frozen=True, slots=True, cache_hash=True + hash=None, eq=True, frozen=True, slots=True, cache_hash=True ) # the cached hash code is stored slightly differently in this case # so it needs to be tested separately @@ -77,23 +79,23 @@ class InitC(object): InitC = _add_init(InitC, False) -class TestAddCmp(object): +class TestEqOrder(object): """ - Tests for `_add_cmp`. + Tests for eq and order related methods. """ @given(booleans()) - def test_cmp(self, slots): + def test_eq_ignore_attrib(self, slots): """ - If `cmp` is False, ignore that attribute. + If `eq` is False for an attribute, ignore that attribute. """ C = make_class( - "C", {"a": attr.ib(cmp=False), "b": attr.ib()}, slots=slots + "C", {"a": attr.ib(eq=False), "b": attr.ib()}, slots=slots ) assert C(1, 2) == C(2, 2) - @pytest.mark.parametrize("cls", [CmpC, CmpCSlots]) + @pytest.mark.parametrize("cls", [EqC, EqCSlots]) def test_equal(self, cls): """ Equal objects are detected as equal. @@ -101,7 +103,7 @@ class TestAddCmp(object): assert cls(1, 2) == cls(1, 2) assert not (cls(1, 2) != cls(1, 2)) - @pytest.mark.parametrize("cls", [CmpC, CmpCSlots]) + @pytest.mark.parametrize("cls", [EqC, EqCSlots]) def test_unequal_same_class(self, cls): """ Unequal objects of correct type are detected as unequal. @@ -109,21 +111,21 @@ class TestAddCmp(object): assert cls(1, 2) != cls(2, 1) assert not (cls(1, 2) == cls(2, 1)) - @pytest.mark.parametrize("cls", [CmpC, CmpCSlots]) + @pytest.mark.parametrize("cls", [EqC, EqCSlots]) def test_unequal_different_class(self, cls): """ Unequal objects of different type are detected even if their attributes match. """ - class NotCmpC(object): + class NotEqC(object): a = 1 b = 2 - assert cls(1, 2) != NotCmpC() - assert not (cls(1, 2) == NotCmpC()) + assert cls(1, 2) != NotEqC() + assert not (cls(1, 2) == NotEqC()) - @pytest.mark.parametrize("cls", [CmpC, CmpCSlots]) + @pytest.mark.parametrize("cls", [OrderC, OrderCSlots]) def test_lt(self, cls): """ __lt__ compares objects as tuples of attribute values. @@ -135,14 +137,14 @@ class TestAddCmp(object): ]: assert cls(*a) < cls(*b) - @pytest.mark.parametrize("cls", [CmpC, CmpCSlots]) + @pytest.mark.parametrize("cls", [OrderC, OrderCSlots]) def test_lt_unordable(self, cls): """ __lt__ returns NotImplemented if classes differ. """ assert NotImplemented == (cls(1, 2).__lt__(42)) - @pytest.mark.parametrize("cls", [CmpC, CmpCSlots]) + @pytest.mark.parametrize("cls", [OrderC, OrderCSlots]) def test_le(self, cls): """ __le__ compares objects as tuples of attribute values. @@ -156,14 +158,14 @@ class TestAddCmp(object): ]: assert cls(*a) <= cls(*b) - @pytest.mark.parametrize("cls", [CmpC, CmpCSlots]) + @pytest.mark.parametrize("cls", [OrderC, OrderCSlots]) def test_le_unordable(self, cls): """ __le__ returns NotImplemented if classes differ. """ assert NotImplemented == (cls(1, 2).__le__(42)) - @pytest.mark.parametrize("cls", [CmpC, CmpCSlots]) + @pytest.mark.parametrize("cls", [OrderC, OrderCSlots]) def test_gt(self, cls): """ __gt__ compares objects as tuples of attribute values. @@ -175,14 +177,14 @@ class TestAddCmp(object): ]: assert cls(*a) > cls(*b) - @pytest.mark.parametrize("cls", [CmpC, CmpCSlots]) + @pytest.mark.parametrize("cls", [OrderC, OrderCSlots]) def test_gt_unordable(self, cls): """ __gt__ returns NotImplemented if classes differ. """ assert NotImplemented == (cls(1, 2).__gt__(42)) - @pytest.mark.parametrize("cls", [CmpC, CmpCSlots]) + @pytest.mark.parametrize("cls", [OrderC, OrderCSlots]) def test_ge(self, cls): """ __ge__ compares objects as tuples of attribute values. @@ -196,7 +198,7 @@ class TestAddCmp(object): ]: assert cls(*a) >= cls(*b) - @pytest.mark.parametrize("cls", [CmpC, CmpCSlots]) + @pytest.mark.parametrize("cls", [OrderC, OrderCSlots]) def test_ge_unordable(self, cls): """ __ge__ returns NotImplemented if classes differ. @@ -347,7 +349,7 @@ class TestAddHash(object): # unhashable case with pytest.raises(TypeError) as e: make_class( - "C", {}, hash=None, cmp=True, frozen=False, cache_hash=True + "C", {}, hash=None, eq=True, frozen=False, cache_hash=True ) assert exc_args == e.value.args @@ -380,13 +382,13 @@ class TestAddHash(object): assert hash(C(1, 2)) == hash(C(2, 2)) @given(booleans()) - def test_hash_attribute_mirrors_cmp(self, cmp): + def test_hash_attribute_mirrors_eq(self, eq): """ - If `hash` is None, the hash generation mirrors `cmp`. + If `hash` is None, the hash generation mirrors `eq`. """ - C = make_class("C", {"a": attr.ib(cmp=cmp)}, cmp=True, frozen=True) + C = make_class("C", {"a": attr.ib(eq=eq)}, eq=True, frozen=True) - if cmp: + if eq: assert C(1) != C(2) assert hash(C(1)) != hash(C(2)) assert hash(C(1)) == hash(C(1)) @@ -395,18 +397,18 @@ class TestAddHash(object): assert hash(C(1)) == hash(C(2)) @given(booleans()) - def test_hash_mirrors_cmp(self, cmp): + def test_hash_mirrors_eq(self, eq): """ - If `hash` is None, the hash generation mirrors `cmp`. + If `hash` is None, the hash generation mirrors `eq`. """ - C = make_class("C", {"a": attr.ib()}, cmp=cmp, frozen=True) + C = make_class("C", {"a": attr.ib()}, eq=eq, frozen=True) i = C(1) assert i == i assert hash(i) == hash(i) - if cmp: + if eq: assert C(1) == C(1) assert hash(C(1)) == hash(C(1)) else: @@ -777,7 +779,7 @@ class TestNothing(object): assert 1 != _Nothing() -@attr.s(hash=True, cmp=True) +@attr.s(hash=True, order=True) class C(object): pass @@ -786,7 +788,7 @@ class C(object): OriginalC = C -@attr.s(hash=True, cmp=True) +@attr.s(hash=True, order=True) class C(object): pass diff --git a/tests/test_funcs.py b/tests/test_funcs.py index eeef8bf2..4d99e06f 100644 --- a/tests/test_funcs.py +++ b/tests/test_funcs.py @@ -25,6 +25,21 @@ MAPPING_TYPES = (dict, OrderedDict) SEQUENCE_TYPES = (list, tuple) +@pytest.fixture(scope="session", name="C") +def fixture_C(): + """ + Return a simple but fully featured attrs class with an x and a y attribute. + """ + import attr + + @attr.s + class C(object): + x = attr.ib() + y = attr.ib() + + return C + + class TestAsDict(object): """ Tests for `asdict`. diff --git a/tests/test_make.py b/tests/test_make.py index 54e37a9a..b90f1c6f 100644 --- a/tests/test_make.py +++ b/tests/test_make.py @@ -14,7 +14,7 @@ from operator import attrgetter import pytest -from hypothesis import given +from hypothesis import assume, given from hypothesis.strategies import booleans, integers, lists, sampled_from, text import attr @@ -28,6 +28,7 @@ from attr._make import ( _Attributes, _ClassBuilder, _CountingAttr, + _determine_eq_order, _transform_attrs, and_, fields, @@ -44,6 +45,7 @@ from attr.exceptions import ( from .strategies import ( gen_attr_names, list_of_attrs, + optional_bool, simple_attrs, simple_attrs_with_metadata, simple_attrs_without_metadata, @@ -216,8 +218,9 @@ class TestTransformAttrs(object): "No mandatory attributes allowed after an attribute with a " "default value or factory. Attribute in question: Attribute" "(name='y', default=NOTHING, validator=None, repr=True, " - "cmp=True, hash=None, init=True, metadata=mappingproxy({}), " - "type=None, converter=None, kw_only=False)", + "eq=True, order=True, hash=None, init=True, " + "metadata=mappingproxy({}), type=None, converter=None, " + "kw_only=False)", ) == e.value.args def test_kw_only(self): @@ -411,21 +414,30 @@ class TestAttributes(object): "arg_name, method_name", [ ("repr", "__repr__"), - ("cmp", "__eq__"), + ("eq", "__eq__"), + ("order", "__le__"), ("hash", "__hash__"), ("init", "__init__"), ], ) def test_respects_add_arguments(self, arg_name, method_name): """ - If a certain `add_XXX` is `False`, `__XXX__` is not added to the class. + If a certain `XXX` is `False`, `__XXX__` is not added to the class. """ # Set the method name to a sentinel and check whether it has been # overwritten afterwards. sentinel = object() - am_args = {"repr": True, "cmp": True, "hash": True, "init": True} + am_args = { + "repr": True, + "eq": True, + "order": True, + "hash": True, + "init": True, + } am_args[arg_name] = False + if arg_name == "eq": + am_args["order"] = False class C(object): x = attr.ib() @@ -918,16 +930,17 @@ class TestFields(object): Tests for `fields`. """ + @given(simple_classes()) def test_instance(self, C): """ Raises `TypeError` on non-classes. """ with pytest.raises(TypeError) as e: - fields(C(1, 2)) + fields(C()) assert "Passed object must be a class." == e.value.args[0] - def test_handler_non_attrs_class(self, C): + def test_handler_non_attrs_class(self): """ Raises `ValueError` if passed a non-``attrs`` instance. """ @@ -959,16 +972,17 @@ class TestFieldsDict(object): Tests for `fields_dict`. """ + @given(simple_classes()) def test_instance(self, C): """ Raises `TypeError` on non-classes. """ with pytest.raises(TypeError) as e: - fields_dict(C(1, 2)) + fields_dict(C()) assert "Passed object must be a class." == e.value.args[0] - def test_handler_non_attrs_class(self, C): + def test_handler_non_attrs_class(self): """ Raises `ValueError` if passed a non-``attrs`` instance. """ @@ -1341,7 +1355,8 @@ class TestClassBuilder(object): ) cls = ( - b.add_cmp() + b.add_eq() + .add_order() .add_hash() .add_init() .add_repr("ns") @@ -1427,7 +1442,7 @@ class TestClassBuilder(object): @attr.s(slots=True) class C(object): __weakref__ = attr.ib( - init=False, hash=False, repr=False, cmp=False + init=False, hash=False, repr=False, eq=False, order=False ) assert C() == copy.deepcopy(C()) @@ -1452,9 +1467,9 @@ class TestClassBuilder(object): assert [C2] == C.__subclasses__() -class TestMakeCmp: +class TestMakeOrder: """ - Tests for _make_cmp(). + Tests for _make_order(). """ def test_subclasses_cannot_be_compared(self): @@ -1500,3 +1515,57 @@ class TestMakeCmp: with pytest.raises(TypeError): a > b + + +class TestDetermineEqOrder(object): + def test_default(self): + """ + If all are set to None, do the default: True, True + """ + assert (True, True) == _determine_eq_order(None, None, None) + + @pytest.mark.parametrize("eq", [True, False]) + def test_order_mirrors_eq_by_default(self, eq): + """ + If order is None, it mirrors eq. + """ + assert (eq, eq) == _determine_eq_order(None, eq, None) + + def test_order_without_eq(self): + """ + eq=False, order=True raises a meaningful ValueError. + """ + with pytest.raises( + ValueError, match="`order` can only be True if `eq` is True too." + ): + _determine_eq_order(None, False, True) + + @given(cmp=booleans(), eq=optional_bool, order=optional_bool) + def test_mix(self, cmp, eq, order): + """ + If cmp is not None, eq and order must be None and vice versa. + """ + assume(eq is not None or order is not None) + + with pytest.raises( + ValueError, match="Don't mix `cmp` with `eq' and `order`." + ): + _determine_eq_order(cmp, eq, order) + + def test_cmp_deprecated(self): + """ + Passing a cmp that is not None raises a DeprecationWarning. + """ + with pytest.deprecated_call() as dc: + + @attr.s(cmp=True) + class C(object): + pass + + w, = dc.list + + assert ( + "The usage of `cmp` is deprecated and will be removed on or after " + "2021-06-01. Please use `eq` and `order` instead." + == w.message.args[0] + ) diff --git a/tests/test_slots.py b/tests/test_slots.py index 3fa6a425..1fda4c1f 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -272,7 +272,9 @@ def test_bare_inheritance_from_slots(): Inheriting from a bare attrs slotted class works. """ - @attr.s(init=False, cmp=False, hash=False, repr=False, slots=True) + @attr.s( + init=False, eq=False, order=False, hash=False, repr=False, slots=True + ) class C1BareSlots(object): x = attr.ib(validator=attr.validators.instance_of(int)) y = attr.ib() @@ -288,7 +290,7 @@ def test_bare_inheritance_from_slots(): def staticmethod(): return "staticmethod" - @attr.s(init=False, cmp=False, hash=False, repr=False) + @attr.s(init=False, eq=False, order=False, hash=False, repr=False) class C1Bare(object): x = attr.ib(validator=attr.validators.instance_of(int)) y = attr.ib() @@ -533,7 +535,9 @@ def tests_weakref_does_not_add_with_weakref_attribute(): @attr.s(slots=True, weakref_slot=True) class C(object): - __weakref__ = attr.ib(init=False, hash=False, repr=False, cmp=False) + __weakref__ = attr.ib( + init=False, hash=False, repr=False, eq=False, order=False + ) c = C() w = weakref.ref(c) diff --git a/tests/typing_example.py b/tests/typing_example.py index a6c48bbc..4df109da 100644 --- a/tests/typing_example.py +++ b/tests/typing_example.py @@ -166,3 +166,10 @@ class WithCustomRepr: b = attr.ib(repr=False) c = attr.ib(repr=lambda value: "c is for cookie") d = attr.ib(repr=str) + + +# Check some of our own types +@attr.s(eq=True, order=False) +class OrderFlags: + a = attr.ib(eq=False, order=False) + b = attr.ib(eq=True, order=True) diff --git a/tests/utils.py b/tests/utils.py index 5a5cf073..86fa3d2b 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -9,7 +9,8 @@ from attr._make import NOTHING, make_class def simple_class( - cmp=False, + eq=False, + order=False, repr=False, hash=False, str=False, @@ -23,7 +24,8 @@ def simple_class( return make_class( "C", ["a", "b"], - cmp=cmp, + eq=eq or order, + order=order, repr=repr, hash=hash, init=True, @@ -39,7 +41,7 @@ def simple_attr( default=NOTHING, validator=None, repr=True, - cmp=True, + eq=True, hash=None, init=True, converter=None, @@ -53,7 +55,8 @@ def simple_attr( default=default, validator=validator, repr=repr, - cmp=cmp, + cmp=None, + eq=eq, hash=hash, init=init, converter=converter,