From 3ef2bf923bf623bbcb5be2e3427d561f0df91ad2 Mon Sep 17 00:00:00 2001 From: Dmitry Malinovsky Date: Wed, 1 Nov 2017 14:16:07 +0600 Subject: [PATCH 1/9] Added support for keyword-only attributes. Closes #106, and closes #38 (Rebases #281) Co-authored-by: Alex Ford --- changelog.d/281.change.rst | 2 + docs/api.rst | 18 +++---- docs/examples.rst | 53 ++++++++++++++++++++- docs/extending.rst | 2 +- src/attr/_make.py | 67 ++++++++++++++++++++++---- tests/test_make.py | 98 +++++++++++++++++++++++++++++++++++++- tests/utils.py | 2 + 7 files changed, 220 insertions(+), 22 deletions(-) create mode 100644 changelog.d/281.change.rst diff --git a/changelog.d/281.change.rst b/changelog.d/281.change.rst new file mode 100644 index 000000000..964e1c8ae --- /dev/null +++ b/changelog.d/281.change.rst @@ -0,0 +1,2 @@ +Added ``kw_only`` argument to ``attr.ib`` and corresponding ``kw_only`` attribute to ``attr.Attribute``. +This change makes it possible to have a generated ``__init__`` with keyword-only arguments on Python 3. diff --git a/docs/api.rst b/docs/api.rst index 3ba23603e..39748dafb 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -90,7 +90,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) + Attribute(name='x', default=NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False) .. autofunction:: attr.make_class @@ -161,9 +161,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), Attribute(name='y', default=NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None)) + (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)) >>> 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) + Attribute(name='y', default=NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False) >>> attr.fields(C).y is attr.fields(C)[1] True @@ -178,9 +178,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), 'y': Attribute(name='y', default=NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None)} + {'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)} >>> 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) + Attribute(name='y', default=NOTHING, validator=None, repr=True, cmp=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 @@ -275,7 +275,7 @@ See :ref:`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), , '1') + 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') 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: @@ -308,11 +308,11 @@ Validators >>> C("42") Traceback (most recent call last): ... - TypeError: ("'x' must be (got '42' that is a ).", Attribute(name='x', default=NOTHING, validator=>, type=None), , '42') + TypeError: ("'x' must be (got '42' that is a ).", Attribute(name='x', default=NOTHING, validator=>, type=None, kw_only=False), , '42') >>> C(None) Traceback (most recent call last): ... - TypeError: ("'x' must be (got None that is a ).", Attribute(name='x', default=NOTHING, validator=>, repr=True, cmp=True, hash=None, init=True, type=None), , None) + TypeError: ("'x' must be (got None that is a ).", Attribute(name='x', default=NOTHING, validator=>, repr=True, cmp=True, hash=None, init=True, type=None, kw_only=False), , None) .. autofunction:: attr.validators.in_ @@ -364,7 +364,7 @@ Validators >>> C("42") Traceback (most recent call last): ... - TypeError: ("'x' must be (got '42' that is a ).", Attribute(name='x', default=NOTHING, validator=>, type=None), , '42') + TypeError: ("'x' must be (got '42' that is a ).", Attribute(name='x', default=NOTHING, validator=>, type=None, kw_only=False), , '42') >>> C(None) C(x=None) diff --git a/docs/examples.rst b/docs/examples.rst index e8014188b..4e8dad74d 100644 --- a/docs/examples.rst +++ b/docs/examples.rst @@ -144,6 +144,55 @@ Therefore ``@attr.s`` comes with the ``repr_ns`` option to set it manually: ``repr_ns`` works on both Python 2 and 3. On Python 3 it overrides the implicit detection. +Keyword-only Attributes +~~~~~~~~~~~~~~~~~~~~~~~ + +When using ``attrs`` on Python 3, you can also add `keyword-only `_ attributes: + +.. doctest:: + + >>> @attr.s + ... class A: + ... a = attr.ib(kw_only=True) + >>> A() + Traceback (most recent call last): + ... + TypeError: A() missing 1 required keyword-only argument: 'a' + >>> A(a=1) + A(a=1) + +If you create an attribute with ``init=False``, ``kw_only`` argument is simply ignored. + +Keyword-only attributes allow subclasses to add attributes without default values, even if the base class defines attributes with default values: + +.. doctest:: + + >>> @attr.s + ... class A: + ... a = attr.ib(default=0) + >>> @attr.s + ... class B(A): + ... b = attr.ib(kw_only=True) + >>> B(b=1) + B(a=0, b=1) + >>> B() + Traceback (most recent call last): + ... + TypeError: B() missing 1 required keyword-only argument: 'b' + +If you omit ``kw_only`` or specify ``kw_only=False``, then you'll get an error: + +.. doctest:: + + >>> @attr.s + ... class A: + ... a = attr.ib(default=0) + >>> @attr.s + ... class B(A): + ... b = attr.ib() + Traceback (most recent call last): + ... + ValueError: No mandatory attributes allowed after an attribute with a default value or factory. Attribute in question: Attribute(name='b', default=NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, convert=None, metadata=mappingproxy({}), type=None, kw_only=False) .. _asdict: @@ -352,7 +401,7 @@ You can use a decorator: >>> C("128") Traceback (most recent call last): ... - TypeError: ("'x' must be (got '128' that is a ).", Attribute(name='x', default=NOTHING, validator=[>, ], repr=True, cmp=True, hash=True, init=True, metadata=mappingproxy({}), type=None, converter=one), , '128') + TypeError: ("'x' must be (got '128' that is a ).", Attribute(name='x', default=NOTHING, validator=[>, ], repr=True, cmp=True, hash=True, init=True, metadata=mappingproxy({}), type=None, converter=one, kw_only=False), , '128') >>> C(256) Traceback (most recent call last): ... @@ -371,7 +420,7 @@ You can use a decorator: >>> C("42") Traceback (most recent call last): ... - TypeError: ("'x' must be (got '42' that is a ).", Attribute(name='x', default=NOTHING, factory=NOTHING, validator=>, type=None), , '42') + TypeError: ("'x' must be (got '42' that is a ).", Attribute(name='x', default=NOTHING, factory=NOTHING, validator=>, type=None, kw_only=False), , '42') Check out :ref:`validators` for more details. diff --git a/docs/extending.rst b/docs/extending.rst index 77f3f6447..11f2a74f4 100644 --- a/docs/extending.rst +++ b/docs/extending.rst @@ -17,7 +17,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),) + (Attribute(name='a', default=NOTHING, validator=None, repr=True, cmp=True, hash=None, init=True, metadata=mappingproxy({}), type=None, converter=None, kw_only=False),) .. warning:: diff --git a/src/attr/_make.py b/src/attr/_make.py index 905f30920..98514c57b 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -79,6 +79,7 @@ def attrib( type=None, converter=None, factory=None, + kw_only=False, ): """ Create a new attribute on a class. @@ -151,6 +152,9 @@ def attrib( This argument is provided for backward compatibility. Regardless of the approach used, the type will be stored on ``Attribute.type``. + :param kw_only: Make this attribute keyword-only (Python 3+) + in the generated ``__init__`` (if ``init`` is ``False``, this + parameter is simply ignored). .. versionadded:: 15.2.0 *convert* .. versionadded:: 16.3.0 *metadata* @@ -163,6 +167,7 @@ def attrib( *convert* to achieve consistency with other noun-based arguments. .. versionadded:: 18.1.0 ``factory=f`` is syntactic sugar for ``default=attr.Factory(f)``. + .. versionadded:: 18.2.0 *kw_only* """ if hash is not None and hash is not True and hash is not False: raise TypeError( @@ -206,6 +211,7 @@ def attrib( converter=converter, metadata=metadata, type=type, + kw_only=kw_only, ) @@ -379,8 +385,15 @@ def _transform_attrs(cls, these, auto_attribs): ) had_default = False + was_kw_only = False for a in attrs: - if had_default is True and a.default is NOTHING and a.init is True: + if ( + was_kw_only is False + and had_default is True + and a.default is NOTHING + and a.init is True + and a.kw_only is False + ): raise ValueError( "No mandatory attributes allowed after an attribute with a " "default value or factory. Attribute in question: %r" % (a,) @@ -389,8 +402,21 @@ def _transform_attrs(cls, these, auto_attribs): had_default is False and a.default is not NOTHING and a.init is not False + and + # Keyword-only attributes without defaults can be specified + # after keyword-only attributes with defaults. + a.kw_only is False ): had_default = True + if was_kw_only is True and a.kw_only is False: + raise ValueError( + "Non keyword-only attributes are not allowed after a " + "keyword-only attribute. Attribute in question: {a!r}".format( + a=a + ) + ) + if was_kw_only is False and a.init is True and a.kw_only is True: + was_kw_only = True return _Attributes((attrs, super_attrs, super_attr_map)) @@ -1298,6 +1324,7 @@ def fmt_setter_with_converter(attr_name, value_var): } args = [] + kw_only_args = [] attrs_to_validate = [] # This is a dictionary of names to validator and converter callables. @@ -1357,11 +1384,13 @@ def fmt_setter_with_converter(attr_name, value_var): ) ) elif a.default is not NOTHING and not has_factory: - args.append( - "{arg_name}=attr_dict['{attr_name}'].default".format( - arg_name=arg_name, attr_name=attr_name - ) + arg = "{arg_name}=attr_dict['{attr_name}'].default".format( + arg_name=arg_name, attr_name=attr_name ) + if a.kw_only: + kw_only_args.append(arg) + else: + args.append(arg) if a.converter is not None: lines.append(fmt_setter_with_converter(attr_name, arg_name)) names_for_globals[ @@ -1370,7 +1399,11 @@ def fmt_setter_with_converter(attr_name, value_var): else: lines.append(fmt_setter(attr_name, arg_name)) elif has_factory: - args.append("{arg_name}=NOTHING".format(arg_name=arg_name)) + arg = "{arg_name}=NOTHING".format(arg_name=arg_name) + if a.kw_only: + kw_only_args.append(arg) + else: + args.append(arg) lines.append( "if {arg_name} is not NOTHING:".format(arg_name=arg_name) ) @@ -1402,7 +1435,10 @@ def fmt_setter_with_converter(attr_name, value_var): ) names_for_globals[init_factory_name] = a.default.factory else: - args.append(arg_name) + if a.kw_only: + kw_only_args.append(arg_name) + else: + args.append(arg_name) if a.converter is not None: lines.append(fmt_setter_with_converter(attr_name, arg_name)) names_for_globals[ @@ -1428,13 +1464,18 @@ def fmt_setter_with_converter(attr_name, value_var): if post_init: lines.append("self.__attrs_post_init__()") + args = ", ".join(args) + if kw_only_args: + args += "{leading_comma}*, {kw_only_args}".format( + leading_comma=", " if args else "", + kw_only_args=", ".join(kw_only_args), + ) return ( """\ def __init__(self, {args}): {lines} """.format( - args=", ".join(args), - lines="\n ".join(lines) if lines else "pass", + args=args, lines="\n ".join(lines) if lines else "pass" ), names_for_globals, annotations, @@ -1463,6 +1504,7 @@ class Attribute(object): "metadata", "type", "converter", + "kw_only", ) def __init__( @@ -1478,6 +1520,7 @@ def __init__( metadata=None, type=None, converter=None, + kw_only=False, ): # Cache this descriptor here to speed things up later. bound_setattr = _obj_setattr.__get__(self, Attribute) @@ -1515,6 +1558,7 @@ def __init__( ), ) bound_setattr("type", type) + bound_setattr("kw_only", kw_only) def __setattr__(self, name, value): raise FrozenInstanceError() @@ -1625,6 +1669,7 @@ class _CountingAttr(object): "_validator", "converter", "type", + "kw_only", ) __attrs_attrs__ = tuple( Attribute( @@ -1635,6 +1680,7 @@ class _CountingAttr(object): cmp=True, hash=True, init=True, + kw_only=False, ) for name in ("counter", "_default", "repr", "cmp", "hash", "init") ) + ( @@ -1646,6 +1692,7 @@ class _CountingAttr(object): cmp=True, hash=False, init=True, + kw_only=False, ), ) cls_counter = 0 @@ -1661,6 +1708,7 @@ def __init__( converter, metadata, type, + kw_only, ): _CountingAttr.cls_counter += 1 self.counter = _CountingAttr.cls_counter @@ -1677,6 +1725,7 @@ def __init__( self.converter = converter self.metadata = metadata self.type = type + self.kw_only = kw_only def validator(self, meth): """ diff --git a/tests/test_make.py b/tests/test_make.py index e407b9f46..2f62c7401 100644 --- a/tests/test_make.py +++ b/tests/test_make.py @@ -280,7 +280,7 @@ class C(object): "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)", + "type=None, converter=None, kw_only=False)", ) == e.value.args def test_these(self): @@ -593,6 +593,102 @@ class C(object): x = attr.ib(factory=Factory(list)) +@pytest.mark.skipif(PY2, reason="keyword-only arguments is PY3-only.") +class TestKeywordOnlyAttributes(object): + """ + Tests for keyword-only attributes. + """ + + def test_adds_keyword_only_arguments(self): + """ + Attributes can be added as keyword-only. + """ + + @attr.s + class C(object): + a = attr.ib() + b = attr.ib(default=2, kw_only=True) + c = attr.ib(kw_only=True) + d = attr.ib(default=attr.Factory(lambda: 4), kw_only=True) + + c = C(1, c=3) + + assert c.a == 1 + assert c.b == 2 + assert c.c == 3 + assert c.d == 4 + + def test_ignores_kw_only_when_init_is_false(self): + """ + Specifying ``kw_only=True`` when ``init=False`` is essentially a no-op. + """ + + @attr.s + class C(object): + x = attr.ib(init=False, default=0, kw_only=True) + y = attr.ib() + + c = C(1) + assert c.x == 0 + assert c.y == 1 + + def test_keyword_only_attributes_presence(self): + """ + Raises `TypeError` when keyword-only arguments are + not specified. + """ + + @attr.s + class C(object): + x = attr.ib(kw_only=True) + + with pytest.raises(TypeError) as e: + C() + + assert ( + "missing 1 required keyword-only argument: 'x'" + ) in e.value.args[0] + + def test_conflicting_keyword_only_attributes(self): + """ + Raises `ValueError` if keyword-only attributes are followed by + regular (non keyword-only) attributes. + """ + + class C(object): + x = attr.ib(kw_only=True) + y = attr.ib() + + with pytest.raises(ValueError) as e: + _transform_attrs(C, None, False) + assert ( + "Non keyword-only attributes are not allowed after a " + "keyword-only attribute. 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)", + ) == e.value.args + + def test_keyword_only_attributes_allow_subclassing(self): + """ + Subclass can define keyword-only attributed without defaults, + when the base class has attributes with defaults. + """ + + @attr.s + class Base(object): + x = attr.ib(default=0) + + @attr.s + class C(Base): + y = attr.ib(kw_only=True) + + c = C(y=1) + + assert c.x == 0 + assert c.y == 1 + + @attr.s class GC(object): @attr.s diff --git a/tests/utils.py b/tests/utils.py index baf73313f..230726c91 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -36,6 +36,7 @@ def simple_attr( hash=None, init=True, converter=None, + kw_only=False, ): """ Return an attribute with a name and no other bells and whistles. @@ -49,6 +50,7 @@ def simple_attr( hash=hash, init=init, converter=converter, + kw_only=False, ) From 8c14ff0739a3797fb27c0a7be933fc0dae7917d5 Mon Sep 17 00:00:00 2001 From: Alex Ford Date: Tue, 24 Jul 2018 01:01:15 -0700 Subject: [PATCH 2/9] Add `attr.s`-level `kw_only` flag. Add `kw_only` flag to `attr.s` decorator, indicating that all class attributes should be keyword-only in __init__. Minor updates to internal interface of `Attribute` to support evolution of attributes to `kw_only` in class factory. Expand examples with `attr.s` level kw_only. --- changelog.d/281.change.rst | 4 +- docs/examples.rst | 17 +++++++ src/attr/_make.py | 47 ++++++++++++----- tests/test_annotations.py | 20 ++++++++ tests/test_make.py | 102 ++++++++++++++++++++++++++++++++----- 5 files changed, 162 insertions(+), 28 deletions(-) diff --git a/changelog.d/281.change.rst b/changelog.d/281.change.rst index 964e1c8ae..e942d18ee 100644 --- a/changelog.d/281.change.rst +++ b/changelog.d/281.change.rst @@ -1,2 +1,2 @@ -Added ``kw_only`` argument to ``attr.ib`` and corresponding ``kw_only`` attribute to ``attr.Attribute``. -This change makes it possible to have a generated ``__init__`` with keyword-only arguments on Python 3. +Added ``kw_only`` arguments to ``attr.ib`` and ``attr.s```, and a corresponding ``kw_only`` attribute to ``attr.Attribute``. +This change makes it possible to have a generated ``__init__`` with keyword-only arguments on Python 3, relaxing the required ordering of default and non-default valued attributes. diff --git a/docs/examples.rst b/docs/examples.rst index 4e8dad74d..d9eeac025 100644 --- a/docs/examples.rst +++ b/docs/examples.rst @@ -161,6 +161,23 @@ When using ``attrs`` on Python 3, you can also add `keyword-only >> A(a=1) A(a=1) +`kw_only` may also be specified at via ``attr.s``, and will apply to all attributes: + +.. doctest:: + + >>> @attr.s(kw_only=True) + ... class A: + ... a = attr.ib() + ... b = attr.ib() + >>> A(1, 2) + Traceback (most recent call last): + ... + TypeError: __init__() takes 1 positional argument but 3 were given + >>> A(a=1, b=2) + A(a=1, b=2) + + + If you create an attribute with ``init=False``, ``kw_only`` argument is simply ignored. Keyword-only attributes allow subclasses to add attributes without default values, even if the base class defines attributes with default values: diff --git a/src/attr/_make.py b/src/attr/_make.py index 98514c57b..a3b50cb75 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -1,5 +1,6 @@ from __future__ import absolute_import, division, print_function +import copy import hashlib import linecache import sys @@ -291,7 +292,7 @@ def _counter_getter(e): return e[1].counter -def _transform_attrs(cls, these, auto_attribs): +def _transform_attrs(cls, these, auto_attribs, kw_only): """ Transform all `_CountingAttr`s on a class into `Attribute`s. @@ -374,15 +375,11 @@ def _transform_attrs(cls, these, auto_attribs): AttrsClass = _make_attr_tuple_class(cls.__name__, attr_names) - attrs = AttrsClass( - super_attrs - + [ - Attribute.from_counting_attr( - name=attr_name, ca=ca, type=anns.get(attr_name) - ) - for attr_name, ca in ca_list - ] - ) + if kw_only: + own_attrs = [a._evolve(kw_only=True) for a in own_attrs] + super_attrs = [a._evolve(kw_only=True) for a in super_attrs] + + attrs = AttrsClass(super_attrs + own_attrs) had_default = False was_kw_only = False @@ -453,9 +450,9 @@ class _ClassBuilder(object): "_super_attr_map", ) - def __init__(self, cls, these, slots, frozen, auto_attribs): + def __init__(self, cls, these, slots, frozen, auto_attribs, kw_only): attrs, super_attrs, super_map = _transform_attrs( - cls, these, auto_attribs + cls, these, auto_attribs, kw_only ) self._cls = cls @@ -665,6 +662,7 @@ def attrs( frozen=False, str=False, auto_attribs=False, + kw_only=False, ): r""" A class decorator that adds `dunder @@ -762,6 +760,10 @@ def attrs( Attributes annotated as :data:`typing.ClassVar` are **ignored**. .. _`PEP 526`: https://www.python.org/dev/peps/pep-0526/ + :param bool kw_only: Make all attributes keyword-only (Python 3+) + in the generated ``__init__`` (if ``init`` is ``False``, this + parameter is simply ignored). + .. versionadded:: 16.0.0 *slots* .. versionadded:: 16.1.0 *frozen* @@ -778,13 +780,16 @@ def attrs( :class:`DeprecationWarning` if the classes compared are subclasses of each other. ``__eq`` and ``__ne__`` never tried to compared subclasses to each other. + .. versionadded:: 18.2.0 *kw_only* """ def wrap(cls): if getattr(cls, "__class__", None) is None: raise TypeError("attrs only works with new-style classes.") - builder = _ClassBuilder(cls, these, slots, frozen, auto_attribs) + builder = _ClassBuilder( + cls, these, slots, frozen, auto_attribs, kw_only + ) if repr is True: builder.add_repr(repr_ns) @@ -1602,6 +1607,17 @@ def from_counting_attr(cls, name, ca, type=None): **inst_dict ) + # Don't use attr.evolve since fields(Attribute) doesn't work + def _evolve(self, **changes): + """ + Create a new instance, based on *self* with *changes* applied. + """ + new = copy.copy(self) + + new._setattrs(changes.items()) + + return new + # Don't use _add_pickle since fields(Attribute) doesn't work def __getstate__(self): """ @@ -1616,8 +1632,11 @@ def __setstate__(self, state): """ Play nice with pickle. """ + self._setattrs(zip(self.__slots__, state)) + + def _setattrs(self, name_values_pairs): bound_setattr = _obj_setattr.__get__(self, Attribute) - for name, value in zip(self.__slots__, state): + for name, value in name_values_pairs: if name != "metadata": bound_setattr(name, value) else: diff --git a/tests/test_annotations.py b/tests/test_annotations.py index 34807a4b7..915a1dccf 100644 --- a/tests/test_annotations.py +++ b/tests/test_annotations.py @@ -229,3 +229,23 @@ class C: "foo": "typing.Any", "return": None, } + + def test_keyword_only_auto_attribs(self): + """ + `kw_only` propagates to attributes defined via `auto_attribs`. + """ + + @attr.s(auto_attribs=True, kw_only=True) + class C: + x: int + y: int + + with pytest.raises(TypeError): + C(0, 1) + + with pytest.raises(TypeError): + C(x=0) + + c = C(x=0, y=1) + assert c.x == 0 + assert c.y == 1 diff --git a/tests/test_make.py b/tests/test_make.py index 2f62c7401..4dcbec795 100644 --- a/tests/test_make.py +++ b/tests/test_make.py @@ -228,7 +228,7 @@ def test_no_modifications(self): Doesn't attach __attrs_attrs__ to the class anymore. """ C = make_tc() - _transform_attrs(C, None, False) + _transform_attrs(C, None, False, False) assert None is getattr(C, "__attrs_attrs__", None) @@ -237,7 +237,7 @@ def test_normal(self): Transforms every `_CountingAttr` and leaves others (a) be. """ C = make_tc() - attrs, _, _ = _transform_attrs(C, None, False) + attrs, _, _ = _transform_attrs(C, None, False, False) assert ["z", "y", "x"] == [a.name for a in attrs] @@ -250,14 +250,16 @@ def test_empty(self): class C(object): pass - assert _Attributes(((), [], {})) == _transform_attrs(C, None, False) + assert _Attributes(((), [], {})) == _transform_attrs( + C, None, False, False + ) def test_transforms_to_attribute(self): """ All `_CountingAttr`s are transformed into `Attribute`s. """ C = make_tc() - attrs, super_attrs, _ = _transform_attrs(C, None, False) + attrs, super_attrs, _ = _transform_attrs(C, None, False, False) assert [] == super_attrs assert 3 == len(attrs) @@ -274,7 +276,7 @@ class C(object): y = attr.ib() with pytest.raises(ValueError) as e: - _transform_attrs(C, None, False) + _transform_attrs(C, None, False, False) assert ( "No mandatory attributes allowed after an attribute with a " "default value or factory. Attribute in question: Attribute" @@ -283,6 +285,36 @@ class C(object): "type=None, converter=None, kw_only=False)", ) == e.value.args + def test_kw_only(self): + """ + Converts all attributes, including superclass attributes, if `kw_only` + is provided. Therefor, `kw_only` allows attributes with defaults to + preceed mandatory attributes. + + Updates in the subclass *don't* affect the superclass attributes. + """ + + @attr.s + class B(object): + b = attr.ib() + + for b_a in B.__attrs_attrs__: + assert b_a.kw_only is False + + class C(B): + x = attr.ib(default=None) + y = attr.ib() + + attrs, super_attrs, _ = _transform_attrs(C, None, False, True) + assert len(attrs) == 3 + assert len(super_attrs) == 1 + + for a in attrs: + assert a.kw_only is True + + for b_a in B.__attrs_attrs__: + assert b_a.kw_only is False + def test_these(self): """ If these is passed, use it and ignore body and super classes. @@ -294,7 +326,9 @@ class Base(object): class C(Base): y = attr.ib() - attrs, super_attrs, _ = _transform_attrs(C, {"x": attr.ib()}, False) + attrs, super_attrs, _ = _transform_attrs( + C, {"x": attr.ib()}, False, False + ) assert [] == super_attrs assert (simple_attr("x"),) == attrs @@ -660,13 +694,13 @@ class C(object): y = attr.ib() with pytest.raises(ValueError) as e: - _transform_attrs(C, None, False) + _transform_attrs(C, None, False, False) assert ( "Non keyword-only attributes are not allowed after a " "keyword-only attribute. 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)", + "cmp=True, hash=None, init=True, metadata=mappingproxy({}), " + "type=None, converter=None, kw_only=False)", ) == e.value.args def test_keyword_only_attributes_allow_subclassing(self): @@ -688,6 +722,45 @@ class C(Base): assert c.x == 0 assert c.y == 1 + def test_keyword_only_class_level(self): + """ + `kw_only` can be provided at the attr.s level, converting all + attributes to `kw_only.` + """ + + @attr.s(kw_only=True) + class C: + x = attr.ib() + y = attr.ib(kw_only=True) + + with pytest.raises(TypeError): + C(0, y=1) + + c = C(x=0, y=1) + assert c.x == 0 + assert c.y == 1 + + def test_keyword_only_class_level_subclassing(self): + """ + Subclass `kw_only` propagates to attrs inherited from the base, + allowing non-default following default. + """ + + @attr.s + class Base(object): + x = attr.ib(default=0) + + @attr.s(kw_only=True) + class C(Base): + y = attr.ib() + + with pytest.raises(TypeError): + C(1) + + c = C(x=0, y=1) + assert c.x == 0 + assert c.y == 1 + @attr.s class GC(object): @@ -1248,7 +1321,7 @@ def test_repr(self): class C(object): pass - b = _ClassBuilder(C, None, True, True, False) + b = _ClassBuilder(C, None, True, True, False, False) assert "<_ClassBuilder(cls=C)>" == repr(b) @@ -1260,7 +1333,7 @@ def test_returns_self(self): class C(object): x = attr.ib() - b = _ClassBuilder(C, None, True, True, False) + b = _ClassBuilder(C, None, True, True, False, False) cls = ( b.add_cmp() @@ -1317,7 +1390,12 @@ class C(object): pass b = _ClassBuilder( - C, these=None, slots=False, frozen=False, auto_attribs=False + C, + these=None, + slots=False, + frozen=False, + auto_attribs=False, + kw_only=False, ) b._cls = {} # no __module__; no __qualname__ From 69ad3aa22dc9dd30b834a3bff24b0579e64a70a4 Mon Sep 17 00:00:00 2001 From: Alex Ford Date: Tue, 24 Jul 2018 01:48:22 -0700 Subject: [PATCH 3/9] Add `kw_only` to type stubs. --- src/attr/__init__.pyi | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/attr/__init__.pyi b/src/attr/__init__.pyi index c8b4c6585..34376a2cf 100644 --- a/src/attr/__init__.pyi +++ b/src/attr/__init__.pyi @@ -56,6 +56,7 @@ class Attribute(Generic[_T]): converter: Optional[_ConverterType[_T]] metadata: Dict[Any, Any] type: Optional[Type[_T]] + kw_only: bool def __lt__(self, x: Attribute) -> bool: ... def __le__(self, x: Attribute) -> bool: ... def __gt__(self, x: Attribute) -> bool: ... @@ -99,6 +100,7 @@ def attrib( type: None = ..., converter: None = ..., factory: None = ..., + kw_only: bool = ..., ) -> Any: ... # This form catches an explicit None or no default and infers the type from the other arguments. @@ -115,6 +117,7 @@ def attrib( type: Optional[Type[_T]] = ..., converter: Optional[_ConverterType[_T]] = ..., factory: Optional[Callable[[], _T]] = ..., + kw_only: bool = ..., ) -> _T: ... # This form catches an explicit default argument. @@ -131,6 +134,7 @@ def attrib( type: Optional[Type[_T]] = ..., converter: Optional[_ConverterType[_T]] = ..., factory: Optional[Callable[[], _T]] = ..., + kw_only: bool = ..., ) -> _T: ... # This form covers type=non-Type: e.g. forward references (str), Any @@ -147,6 +151,7 @@ def attrib( type: object = ..., converter: Optional[_ConverterType[_T]] = ..., factory: Optional[Callable[[], _T]] = ..., + kw_only: bool = ..., ) -> Any: ... @overload def attrs( @@ -161,6 +166,7 @@ def attrs( frozen: bool = ..., str: bool = ..., auto_attribs: bool = ..., + kw_only: bool = ..., ) -> _C: ... @overload def attrs( @@ -175,6 +181,7 @@ def attrs( frozen: bool = ..., str: bool = ..., auto_attribs: bool = ..., + kw_only: bool = ..., ) -> Callable[[_C], _C]: ... # TODO: add support for returning NamedTuple from the mypy plugin @@ -200,6 +207,7 @@ def make_class( frozen: bool = ..., str: bool = ..., auto_attribs: bool = ..., + kw_only: bool = ..., ) -> type: ... # _funcs -- From 117b81ef3932c74d4ac4efc7cab7cf66fe4a193f Mon Sep 17 00:00:00 2001 From: Alex Ford Date: Tue, 24 Jul 2018 10:53:04 -0700 Subject: [PATCH 4/9] Update changelog for rebased PR. Hear ye, hear ye. A duplicate PR is born. --- changelog.d/411.change.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelog.d/411.change.rst diff --git a/changelog.d/411.change.rst b/changelog.d/411.change.rst new file mode 100644 index 000000000..e942d18ee --- /dev/null +++ b/changelog.d/411.change.rst @@ -0,0 +1,2 @@ +Added ``kw_only`` arguments to ``attr.ib`` and ``attr.s```, and a corresponding ``kw_only`` attribute to ``attr.Attribute``. +This change makes it possible to have a generated ``__init__`` with keyword-only arguments on Python 3, relaxing the required ordering of default and non-default valued attributes. From b1035ee559b9f4b3e14c4a8306487d5aaac63e17 Mon Sep 17 00:00:00 2001 From: Alex Ford Date: Sat, 28 Jul 2018 15:27:30 -0700 Subject: [PATCH 5/9] Tidy docs from review. --- docs/examples.rst | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/examples.rst b/docs/examples.rst index 11aa76bca..656d969f3 100644 --- a/docs/examples.rst +++ b/docs/examples.rst @@ -144,6 +144,7 @@ Therefore ``@attr.s`` comes with the ``repr_ns`` option to set it manually: ``repr_ns`` works on both Python 2 and 3. On Python 3 it overrides the implicit detection. + Keyword-only Attributes ~~~~~~~~~~~~~~~~~~~~~~~ @@ -161,7 +162,7 @@ When using ``attrs`` on Python 3, you can also add `keyword-only >> A(a=1) A(a=1) -`kw_only` may also be specified at via ``attr.s``, and will apply to all attributes: +``kw_only`` may also be specified at via ``attr.s``, and will apply to all attributes: .. doctest:: @@ -178,7 +179,7 @@ When using ``attrs`` on Python 3, you can also add `keyword-only Date: Sat, 28 Jul 2018 15:27:52 -0700 Subject: [PATCH 6/9] Tidy code from review. --- src/attr/_make.py | 6 +++--- tests/test_annotations.py | 1 + tests/test_make.py | 9 +++++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index 8fabf9ea8..efff8e781 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -155,7 +155,7 @@ def attrib( ``Attribute.type``. :param kw_only: Make this attribute keyword-only (Python 3+) in the generated ``__init__`` (if ``init`` is ``False``, this - parameter is simply ignored). + parameter is ignored). .. versionadded:: 15.2.0 *convert* .. versionadded:: 16.3.0 *metadata* @@ -168,7 +168,7 @@ def attrib( *convert* to achieve consistency with other noun-based arguments. .. versionadded:: 18.1.0 ``factory=f`` is syntactic sugar for ``default=attr.Factory(f)``. - .. versionadded:: 18.2.0 *kw_only* + .. versionadded:: 18.2.0 *kw_only* """ if hash is not None and hash is not True and hash is not False: raise TypeError( @@ -762,7 +762,7 @@ def attrs( .. _`PEP 526`: https://www.python.org/dev/peps/pep-0526/ :param bool kw_only: Make all attributes keyword-only (Python 3+) in the generated ``__init__`` (if ``init`` is ``False``, this - parameter is simply ignored). + parameter is ignored). .. versionadded:: 16.0.0 *slots* diff --git a/tests/test_annotations.py b/tests/test_annotations.py index 915a1dccf..fee45d180 100644 --- a/tests/test_annotations.py +++ b/tests/test_annotations.py @@ -247,5 +247,6 @@ class C: C(x=0) c = C(x=0, y=1) + assert c.x == 0 assert c.y == 1 diff --git a/tests/test_make.py b/tests/test_make.py index e48c50e6a..e56321160 100644 --- a/tests/test_make.py +++ b/tests/test_make.py @@ -289,7 +289,7 @@ class C(object): def test_kw_only(self): """ Converts all attributes, including superclass attributes, if `kw_only` - is provided. Therefor, `kw_only` allows attributes with defaults to + is provided. Therefore, `kw_only` allows attributes with defaults to preceed mandatory attributes. Updates in the subclass *don't* affect the superclass attributes. @@ -307,6 +307,7 @@ class C(B): y = attr.ib() attrs, super_attrs, _ = _transform_attrs(C, None, False, True) + assert len(attrs) == 3 assert len(super_attrs) == 1 @@ -628,7 +629,7 @@ class C(object): x = attr.ib(factory=Factory(list)) -@pytest.mark.skipif(PY2, reason="keyword-only arguments is PY3-only.") +@pytest.mark.skipif(PY2, reason="keyword-only arguments are PY3-only.") class TestKeywordOnlyAttributes(object): """ Tests for keyword-only attributes. @@ -664,6 +665,7 @@ class C(object): y = attr.ib() c = C(1) + assert c.x == 0 assert c.y == 1 @@ -696,6 +698,7 @@ class C(object): with pytest.raises(ValueError) as e: _transform_attrs(C, None, False, False) + assert ( "Non keyword-only attributes are not allowed after a " "keyword-only attribute. Attribute in question: Attribute" @@ -738,6 +741,7 @@ class C: C(0, y=1) c = C(x=0, y=1) + assert c.x == 0 assert c.y == 1 @@ -759,6 +763,7 @@ class C(Base): C(1) c = C(x=0, y=1) + assert c.x == 0 assert c.y == 1 From 42ec86d72de6b7fdfe86f4731f2dc161db79518a Mon Sep 17 00:00:00 2001 From: Alex Ford Date: Sat, 28 Jul 2018 16:12:50 -0700 Subject: [PATCH 7/9] Add explicit tests of PY2 kw_only SyntaxError behavior. --- tests/test_make.py | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/test_make.py b/tests/test_make.py index e56321160..2aa68b5e2 100644 --- a/tests/test_make.py +++ b/tests/test_make.py @@ -768,6 +768,43 @@ class C(Base): assert c.y == 1 +@pytest.mark.skipif(not PY2, reason="PY2-specific keyword-only error behavior") +class TestKeywordOnlyAttributesOnPy2(object): + """ + Tests for keyword-only attribute behavior on py2. + """ + + def test_syntax_error(self): + """ + Keyword-only attributes raise Syntax error on ``__init__`` generation. + """ + + with pytest.raises(SyntaxError): + + @attr.s(kw_only=True) + class ClassLevel(object): + a = attr.ib() + + with pytest.raises(SyntaxError): + + @attr.s() + class AttrLevel(object): + a = attr.ib(kw_only=True) + + def test_no_init(self): + """ + Keyworld-only is a no-op, not any error, if ``init=false``. + """ + + @attr.s(kw_only=True, init=False) + class ClassLevel(object): + a = attr.ib() + + @attr.s(init=False) + class AttrLevel(object): + a = attr.ib(kw_only=True) + + @attr.s class GC(object): @attr.s From 69011e122cc426b4b8caedc9fac090906ee1b3f0 Mon Sep 17 00:00:00 2001 From: Alex Ford Date: Tue, 31 Jul 2018 11:03:41 -0700 Subject: [PATCH 8/9] Add `PythonToOldError`, raise for kw_only on PY2. --- src/attr/_make.py | 6 ++++++ src/attr/exceptions.py | 8 ++++++++ tests/test_make.py | 10 +++++++--- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index efff8e781..0163142c6 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -22,6 +22,7 @@ DefaultAlreadySetError, FrozenInstanceError, NotAnAttrsClassError, + PythonTooOldError, UnannotatedAttributeError, ) @@ -1471,6 +1472,11 @@ def fmt_setter_with_converter(attr_name, value_var): args = ", ".join(args) if kw_only_args: + if PY2: + raise PythonTooOldError( + "Keyword-only arguments only work on Python 3 and later." + ) + args += "{leading_comma}*, {kw_only_args}".format( leading_comma=", " if args else "", kw_only_args=", ".join(kw_only_args), diff --git a/src/attr/exceptions.py b/src/attr/exceptions.py index 1a3229f53..b12e41e97 100644 --- a/src/attr/exceptions.py +++ b/src/attr/exceptions.py @@ -47,3 +47,11 @@ class UnannotatedAttributeError(RuntimeError): .. versionadded:: 17.3.0 """ + + +class PythonTooOldError(RuntimeError): + """ + An ``attrs`` feature requiring a more recent python version has been used. + + .. versionadded:: 18.2.0 + """ diff --git a/tests/test_make.py b/tests/test_make.py index 2aa68b5e2..0302cb5f3 100644 --- a/tests/test_make.py +++ b/tests/test_make.py @@ -35,7 +35,11 @@ make_class, validate, ) -from attr.exceptions import DefaultAlreadySetError, NotAnAttrsClassError +from attr.exceptions import ( + DefaultAlreadySetError, + NotAnAttrsClassError, + PythonTooOldError, +) from .strategies import ( gen_attr_names, @@ -779,13 +783,13 @@ def test_syntax_error(self): Keyword-only attributes raise Syntax error on ``__init__`` generation. """ - with pytest.raises(SyntaxError): + with pytest.raises(PythonTooOldError): @attr.s(kw_only=True) class ClassLevel(object): a = attr.ib() - with pytest.raises(SyntaxError): + with pytest.raises(PythonTooOldError): @attr.s() class AttrLevel(object): From d8eb3a2a8d7d0578ccde7c1341e713a735e69c3d Mon Sep 17 00:00:00 2001 From: Alex Ford Date: Wed, 1 Aug 2018 08:32:44 -0700 Subject: [PATCH 9/9] `Attribute._evolve` to `Attribute._assoc`. --- src/attr/_make.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index 0163142c6..71c0f23b7 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -377,8 +377,8 @@ def _transform_attrs(cls, these, auto_attribs, kw_only): AttrsClass = _make_attr_tuple_class(cls.__name__, attr_names) if kw_only: - own_attrs = [a._evolve(kw_only=True) for a in own_attrs] - super_attrs = [a._evolve(kw_only=True) for a in super_attrs] + own_attrs = [a._assoc(kw_only=True) for a in own_attrs] + super_attrs = [a._assoc(kw_only=True) for a in super_attrs] attrs = AttrsClass(super_attrs + own_attrs) @@ -1613,10 +1613,10 @@ def from_counting_attr(cls, name, ca, type=None): **inst_dict ) - # Don't use attr.evolve since fields(Attribute) doesn't work - def _evolve(self, **changes): + # Don't use attr.assoc since fields(Attribute) doesn't work + def _assoc(self, **changes): """ - Create a new instance, based on *self* with *changes* applied. + Copy *self* and apply *changes*. """ new = copy.copy(self)