Repository: python-attrs/attrs
Pull request #773 (2021-02-27 11:40:20)

Undeprecate cmp
# Pull Request Check List

This is just a friendly reminder about the most common mistakes.  Please make sure that you tick all boxes.  But please read our [contribution guide](https://www.attrs.org/en/latest/contributing.html) at least once, it will save you unnecessary review cycles!

If an item doesn't apply to your pull request, **check it anyway** to make it apparent that there's nothing left to do.  If your pull request is a documentation fix or a trivial typo, feel free to delete the whole thing.

- [x] Removed **tests** for cmp deprecation warning.
- [x] Updated **documentation** for changed code.
    - [x] Changed/added classes/methods/functions have appropriate `versionadded`, `versionchanged`, or `deprecated` [directives](http://www.sphinx-doc.org/en/stable/markup/para.html#directive-versionadded).  Find the appropriate next version in our [``__init__.py``](https://github.com/python-attrs/attrs/blob/main/src/attr/__init__.py) file.
- [x] Documentation in `.rst` files is written using [semantic newlines](https://rhodesmill.org/brandon/2012/one-sentence-per-line/).
- [x] Changes (and possible deprecations) have news fragments in [`changelog.d`](https://github.com/python-attrs/attrs/blob/main/changelog.d).

If you have *any* questions to *any* of the points above, just **submit and ask**!  This checklist is here to *help* you, not to deter you from contributing!


botant: Not sure which changelog file I should add. I thought it was 773, but there is already a 787.
hynek: > Not sure which changelog file I should add. I thought it was 773, but there is already a 787.

I'm confused, what is about 787? :) Your correct issue number is 773 so make it `773.deprecation.rst`

Don't spend too much time on it, I will edit it anyways because I need to add some apologies.
botant: > I'm confused, what is about 787? :) Your correct issue number is 773 so make it `773.deprecation.rst`

Done.

botant: Out of curiosity, what is keeping us from removing `cmp` from `Attribute`'s `init`? As far as I understand, attributes are not supposed to be instantiated manually, and `cmp` is already not used there anyway.

How about the property? Is it even possible to quantity the amount of damage that would be caused if we removed it?

Just curious to understand how you manage breaking changes.

hynek: > Out of curiosity, what is keeping us from removing `cmp` from `Attribute`'s `init`? As far as I understand, attributes are not supposed to be instantiated manually, and `cmp` is already not used there anyway.

Yes, but people have historically done so nevertheless. :(

> How about the property? Is it even possible to quantity the amount of damage that would be caused if we removed it?

It's not! Which is why SemVer is bullshit (blog post incoming) but I'm trying to be extra careful and diligent.

They're both gonna move on to a better place in June.

> Just curious to understand how you manage breaking changes.

Poorly, but I'm trying my best. :)

Patch:
diff --git a/changelog.d/773.deprecation.rst b/changelog.d/773.deprecation.rst
new file mode 100644
index 0000000..2b694d3
--- /dev/null
+++ b/changelog.d/773.deprecation.rst
@@ -0,0 +1,1 @@
+``cmp`` has been undeprecated and will continue to be supported as syntactic sugar to set ``eq`` and ``order`` in one go.
diff --git a/docs/api.rst b/docs/api.rst
index 08067d5..cfda7ba 100644
--- a/docs/api.rst
+++ b/docs/api.rst
@@ -653,6 +653,4 @@ It behaves similarly to `sys.version_info` and is an instance of `VersionInfo`:
 The serious business aliases used to be called ``attr.attributes`` and ``attr.attr``.
 There are no plans to remove them but they shouldn't be used in new code.
 
-The ``cmp`` argument to both `attr.s` and `attr.ib` has been deprecated in 19.2 and shouldn't be used.
-
 .. autofunction:: assoc
diff --git a/src/attr/_make.py b/src/attr/_make.py
index ee70160..3597dd5 100644
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -196,7 +196,6 @@ def attrib(
 
     :param cmp: Setting to ``True`` is equivalent to setting ``eq=True,
         order=True``. Can also be set to a ``callable``.
-        Deprecated in favor of *eq* and *order*.
     :type cmp: a `bool` or a `callable`.
 
     :param Optional[bool] hash: Include this attribute in the generated
@@ -252,7 +251,8 @@ def attrib(
     .. versionadded:: 19.2.0 *eq* and *order*
     .. versionadded:: 20.1.0 *on_setattr*
     .. versionchanged:: 20.3.0 *kw_only* backported to Python 2
-    .. versionchanged:: 21.1.0 *eq* and *order* also accept a custom callable.
+    .. versionchanged:: 21.1.0 *eq* and *order* also accept a custom callable
+    .. versionchanged:: 21.1.0 *cmp* undeprecated
     """
     eq, eq_key, order, order_key = _determine_attrib_eq_order(
         cmp, eq, order, True
@@ -1083,8 +1083,6 @@ def _determine_attrs_eq_order(cmp, eq, order, default_eq):
 
     # 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 set to the specified default and ordering
@@ -1121,8 +1119,6 @@ def _determine_attrib_eq_order(cmp, eq, order, default_eq):
 
     # cmp takes precedence due to bw-compatibility.
     if cmp is not None:
-        warnings.warn(_CMP_DEPRECATION, DeprecationWarning, stacklevel=3)
-
         cmp, cmp_key = decide_callable_or_boolean(cmp)
         return cmp, cmp_key, cmp, cmp_key
 
@@ -1263,9 +1259,7 @@ def attrs(
         allow instances to be ordered. If ``None`` (default) mirror value of
         *eq*.
     :param Optional[bool] 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*.
+        ``eq=True, order=True``. Must not be mixed with *eq* or *order*.
     :param Optional[bool] hash: If ``None`` (default), the ``__hash__`` method
         is generated according how *eq* and *frozen* are set.
 
@@ -1438,7 +1432,7 @@ def attrs(
     .. versionchanged:: 21.1.0
        ``init=False`` injects ``__attrs_init__``
     .. versionchanged:: 21.1.0 Support for ``__attrs_pre_init__``
-
+    .. versionchanged:: 21.1.0 *cmp* undeprecated
     """
     if auto_detect and PY2:
         raise PythonTooOldError(
@@ -2762,7 +2756,7 @@ class _CountingAttr(object):
         default,
         validator,
         repr,
-        cmp,  # XXX: unused, remove along with cmp
+        cmp,
         hash,
         init,
         converter,


Test patch:
diff --git a/tests/test_functional.py b/tests/test_functional.py
index 3878d68..a17023f 100644
--- a/tests/test_functional.py
+++ b/tests/test_functional.py
@@ -680,10 +680,6 @@ class TestFunctional(object):
 
             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 (
diff --git a/tests/test_make.py b/tests/test_make.py
index 9486cfe..486f53f 100644
--- a/tests/test_make.py
+++ b/tests/test_make.py
@@ -1862,24 +1862,6 @@ class TestDetermineAttrsEqOrder(object):
         ):
             _determine_attrs_eq_order(cmp, eq, order, True)
 
-    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]
-        )
-
 
 class TestDetermineAttribEqOrder(object):
     def test_default(self):
@@ -1968,45 +1950,6 @@ class TestDetermineAttribEqOrder(object):
         ):
             _determine_attrib_eq_order(cmp, eq, order, True)
 
-    def test_boolean_cmp_deprecated(self):
-        """
-        Passing a cmp that is not None raises a DeprecationWarning.
-        """
-        with pytest.deprecated_call() as dc:
-
-            assert (True, None, True, None) == _determine_attrib_eq_order(
-                True, None, None, True
-            )
-
-        (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]
-        )
-
-    def test_callable_cmp_deprecated(self):
-        """
-        Passing a cmp that is not None raises a DeprecationWarning.
-        """
-        with pytest.deprecated_call() as dc:
-
-            assert (
-                True,
-                str.lower,
-                True,
-                str.lower,
-            ) == _determine_attrib_eq_order(str.lower, None, None, True)
-
-        (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]
-        )
-
 
 class TestDocs:
     @pytest.mark.parametrize(
@@ -2301,7 +2244,7 @@ class TestAutoDetect:
             (None, None, True),
         ],
     )
-    def test_override_order(self, slots, frozen, eq, order, cmp, recwarn):
+    def test_override_order(self, slots, frozen, eq, order, cmp):
         """
         If order=True is passed, ignore __le__, __lt__, __gt__, __ge__.
 
@@ -2328,11 +2271,6 @@ class TestAutoDetect:
         assert C(2) > C(1)
         assert C(2) >= C(1)
 
-        if cmp:
-            assert 1 == len(recwarn.list)
-        else:
-            assert 0 == len(recwarn.list)
-
     @pytest.mark.parametrize("slots", [True, False])
     @pytest.mark.parametrize("first", [True, False])
     def test_total_ordering(self, slots, first):