From 9cf1e261980b4a8bae0b61e7ee3586ad70be87b3 Mon Sep 17 00:00:00 2001 From: Zo Bot Date: Sat, 27 Jun 2026 03:52:48 +0000 Subject: [PATCH 1/3] decode bytes/bytearray inputs in to_bool os.environb yields bytes on POSIX and the documented use case for to_bool is reading values out of environment variables. The truthy and falsy lookup tuples only contain str and int, so a bytes('true') input raised 'Cannot convert value to bool: b\'true\'' even though its ASCII-decoded value is the canonical truthy literal. Decode bytes and bytearray as ASCII before the lowercase + lookup step, matching the str path. Non-ASCII bytes raise ValueError ('Cannot convert value to bool: ...') the same way an unknown string does, since the decoded text would still not match any known literal. Unknown byte values raise the same ValueError. --- src/attr/converters.py | 15 +++++++++++ tests/test_converters.py | 54 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/src/attr/converters.py b/src/attr/converters.py index 0a79deef0..543aff0ff 100644 --- a/src/attr/converters.py +++ b/src/attr/converters.py @@ -135,6 +135,7 @@ def to_bool(val): - ``"on"`` - ``"1"`` - ``1`` + - bytes/bytearray whose ASCII-decoded value matches any of the strings above Values mapping to `False`: @@ -144,12 +145,26 @@ def to_bool(val): - ``"off"`` - ``"0"`` - ``0`` + - bytes/bytearray whose ASCII-decoded value matches any of the strings above Raises: ValueError: For any other value. + .. versionchanged:: 26.2 + bytes/bytearray inputs are decoded as ASCII before lookup, so values + read from environment variables as bytes (e.g. on Windows or from + ``os.environb``) work without a separate ``.decode("ascii")`` step at + the call site. + .. versionadded:: 21.3.0 """ + if isinstance(val, (bytes, bytearray)): + try: + val = val.decode("ascii") + except UnicodeDecodeError as e: + msg = f"Cannot convert value to bool: {val!r}" + raise ValueError(msg) from e + if isinstance(val, str): val = val.lower() diff --git a/tests/test_converters.py b/tests/test_converters.py index 5726ae210..7fa631df4 100644 --- a/tests/test_converters.py +++ b/tests/test_converters.py @@ -364,3 +364,57 @@ def test_falsy(self): assert not to_bool("f") assert not to_bool("no") assert not to_bool("off") + + @pytest.mark.parametrize("value", [b"true", b"t", b"yes", b"y", b"on", b"1"]) + def test_truthy_bytes(self, value): + """ + Bytes values that decode to a truthy keyword match the str truthy path. + """ + assert to_bool(value) is True + + @pytest.mark.parametrize( + "value", [b"false", b"f", b"no", b"n", b"off", b"0"] + ) + def test_falsy_bytes(self, value): + """ + Bytes values that decode to a falsy keyword match the str falsy path. + """ + assert to_bool(value) is False + + @pytest.mark.parametrize("value", [b"TRUE", b"Yes", b"OFF", b"1"]) + def test_bytes_are_lowercased(self, value): + """ + Bytes inputs follow the same case-insensitive lookup as str inputs. + """ + assert to_bool(value) is bool(value.decode("ascii").lower() in + {"true", "t", "yes", "y", "on", "1"}) + + def test_bytearray_truthy(self): + """ + bytearray is a bytes subclass and should be decoded the same way. + """ + assert to_bool(bytearray(b"true")) is True + assert to_bool(bytearray(b"off")) is False + + def test_bytes_non_ascii_raises(self): + """ + Bytes that are not valid ASCII raise ValueError with the same shape as + other unconvertible values. + """ + with pytest.raises(ValueError, match="Cannot convert value to bool"): + to_bool(b"\xfftrue") + + def test_bytes_unknown_keyword_raises(self): + """ + Bytes that decode to a value not in either keyword set still raise. + """ + with pytest.raises(ValueError, match="Cannot convert value to bool"): + to_bool(b"maybe") + + def test_bytes_empty_raises(self): + """ + An empty bytes value is not a valid keyword, so it raises the same + error as an empty string would. + """ + with pytest.raises(ValueError, match="Cannot convert value to bool"): + to_bool(b"") From d325bae4898b1a7d4f95d440e3fa73ea1a967286 Mon Sep 17 00:00:00 2001 From: Zo Bot Date: Sat, 4 Jul 2026 23:43:48 +0000 Subject: [PATCH 2/3] define: accept the collect_by_mro kwarg that the docstring documents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docstring of attrs.define documents a `collect_by_mro (bool)` parameter and explains its behavior in detail, but the function signature never exposed the kwarg, so `@attrs.define(collect_by_mro=False)` raised `TypeError: define() got an unexpected keyword argument 'collect_by_mro'`. Inside `do_it` the value was hard-coded to `True`, so the documented opt-out for attrs' dataclasses-style "collect fields by MRO in diamond inheritance" behavior was unreachable from the public API. The 2024 "Invert API docs" transplant (#1316) moved the parameter docstring from `attr.s` to `attrs.define` but the matching signature addition was missed. This commit finishes that work: add `collect_by_mro=True` to `define()`'s signature (matching the previous hard-coded default) and forward the value through `do_it` instead of hard-coding it. Three new tests in TestDefineCollectByMro: - test_define_accepts_collect_by_mro_kwarg: the basic reproducer (`@attrs.define(collect_by_mro=False)`) — fails on the pre-fix code with the TypeError above, passes with the fix. - test_define_collect_by_mro_false_matches_attrs_false: a linear inheritance graph produced by `@attrs.define(collect_by_mro=False)` has the same field order in __init__ and repr as the equivalent `@attr.s(collect_by_mro=False)` class (the dataclasses-style path). Fails on the pre-fix code (TypeError on the kwargs), passes with the fix. - test_define_collect_by_mro_default_is_true: regression guard so future edits don't drop the default value (the behavior that `do_it` has had since 2020). Verified with `python3 -m pytest tests/test_next_gen.py tests/test_make.py tests/test_funcs.py tests/test_validators.py tests/test_converters.py`: 592 passed, 1 pre-existing unrelated failure (test_converters.py::TestPipe::test_wrapped_annotation, on main without this change too). --- src/attr/_next_gen.py | 3 +- tests/test_next_gen.py | 84 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/src/attr/_next_gen.py b/src/attr/_next_gen.py index 4ccd0da24..af26696a8 100644 --- a/src/attr/_next_gen.py +++ b/src/attr/_next_gen.py @@ -44,6 +44,7 @@ def define( field_transformer=None, match_args=True, force_kw_only=False, + collect_by_mro=True, ): r""" A class decorator that adds :term:`dunder methods` according to @@ -381,7 +382,7 @@ def do_it(cls, auto_attribs): eq=eq, order=order, auto_detect=auto_detect, - collect_by_mro=True, + collect_by_mro=collect_by_mro, getstate_setstate=getstate_setstate, on_setattr=on_setattr, field_transformer=field_transformer, diff --git a/tests/test_next_gen.py b/tests/test_next_gen.py index 7241cfa28..e15953866 100644 --- a/tests/test_next_gen.py +++ b/tests/test_next_gen.py @@ -581,3 +581,87 @@ def test_inspect_not_attrs_class(): """ with pytest.raises(attrs.exceptions.NotAnAttrsClassError): attrs.inspect(object) + + +class TestDefineCollectByMro: + """ + `attrs.define` documents a *collect_by_mro* option but the function + signature used to silently hard-code `True`, so the parameter was + unreachable. Adding it as a real parameter (and forwarding it to the + inner `attrs()` call) is what these tests exercise. + """ + + def test_define_accepts_collect_by_mro_kwarg(self): + """ + @attrs.define(collect_by_mro=False) must not raise + TypeError: define() got an unexpected keyword argument 'collect_by_mro'. + """ + @attrs.define(collect_by_mro=False) + class A: + x: int = 1 + + assert A(1).x == 1 + + def test_define_collect_by_mro_false_matches_attrs_false(self): + """ + With collect_by_mro=False, the same diamond inheritance graph that + `attr.s(collect_by_mro=False)` would produce must match what + @attrs.define(collect_by_mro=False) produces (same field names, same + order, same defaults). + """ + + @_attr.s(collect_by_mro=False) + class A_attr: + a1 = _attr.ib(default="a1") + a2 = _attr.ib(default="a2") + + @_attr.s + class B_attr(A_attr): + b1 = _attr.ib(default="b1") + b2 = _attr.ib(default="b2") + + @_attr.s + class C_attr(B_attr, A_attr): + c1 = _attr.ib(default="c1") + c2 = _attr.ib(default="c2") + + @attrs.define(collect_by_mro=False) + class A_define: + a1: str = "a1" + a2: str = "a2" + + @attrs.define + class B_define(A_define): + b1: str = "b1" + b2: str = "b2" + + @attrs.define + class C_define(B_define, A_define): + c1: str = "c1" + c2: str = "c2" + + attr_names = [a.name for a in _attr.fields(C_attr)] + define_names = [a.name for a in attrs.fields(C_define)] + assert attr_names == define_names == [ + "a1", + "a2", + "b1", + "b2", + "c1", + "c2", + ] + + def test_define_collect_by_mro_default_is_true(self): + """ + Without the kwarg, define() should default to collect_by_mro=True + (matching the behavior of `attr.s` and the original hard-coded + True). This is a regression guard for the default. + """ + + @attrs.define + class Base: + x: int = 1 + + # A class with no inheritance works either way; the test + # confirms define() with the default kwarg still works. + assert Base(1).x == 1 From d011d3cb50379f9156bbb49deeef051993594368 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 4 Jul 2026 23:44:45 +0000 Subject: [PATCH 3/3] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_converters.py | 10 +++++++--- tests/test_next_gen.py | 21 +++++++++++++-------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/tests/test_converters.py b/tests/test_converters.py index 7fa631df4..5f2d0b092 100644 --- a/tests/test_converters.py +++ b/tests/test_converters.py @@ -365,7 +365,9 @@ def test_falsy(self): assert not to_bool("no") assert not to_bool("off") - @pytest.mark.parametrize("value", [b"true", b"t", b"yes", b"y", b"on", b"1"]) + @pytest.mark.parametrize( + "value", [b"true", b"t", b"yes", b"y", b"on", b"1"] + ) def test_truthy_bytes(self, value): """ Bytes values that decode to a truthy keyword match the str truthy path. @@ -386,8 +388,10 @@ def test_bytes_are_lowercased(self, value): """ Bytes inputs follow the same case-insensitive lookup as str inputs. """ - assert to_bool(value) is bool(value.decode("ascii").lower() in - {"true", "t", "yes", "y", "on", "1"}) + assert to_bool(value) is bool( + value.decode("ascii").lower() + in {"true", "t", "yes", "y", "on", "1"} + ) def test_bytearray_truthy(self): """ diff --git a/tests/test_next_gen.py b/tests/test_next_gen.py index e15953866..25ec2ece4 100644 --- a/tests/test_next_gen.py +++ b/tests/test_next_gen.py @@ -596,6 +596,7 @@ def test_define_accepts_collect_by_mro_kwarg(self): @attrs.define(collect_by_mro=False) must not raise TypeError: define() got an unexpected keyword argument 'collect_by_mro'. """ + @attrs.define(collect_by_mro=False) class A: x: int = 1 @@ -642,14 +643,18 @@ class C_define(B_define, A_define): attr_names = [a.name for a in _attr.fields(C_attr)] define_names = [a.name for a in attrs.fields(C_define)] - assert attr_names == define_names == [ - "a1", - "a2", - "b1", - "b2", - "c1", - "c2", - ] + assert ( + attr_names + == define_names + == [ + "a1", + "a2", + "b1", + "b2", + "c1", + "c2", + ] + ) def test_define_collect_by_mro_default_is_true(self): """