From c35e03ca1d35408085c418951f18198eb590dc0d Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Tue, 5 Sep 2023 09:29:35 +0100 Subject: [PATCH 01/11] feat: add `repr` kwarg to `field` function and `Field` C struct Extended the field Python function to accept a new keyword argument `repr` which controls the inclusion of the field in the generated `__repr__` string for the struct. Added a new attribute `repr` to the `Field` C struct to store the value of the `repr` keyword argument. Updated the `Field` C struct docstring to include details about the new `repr` kwarg. --- msgspec/__init__.py | 6 ++++-- msgspec/_core.c | 7 +++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/msgspec/__init__.py b/msgspec/__init__.py index 2bd99f58..e2a22f62 100644 --- a/msgspec/__init__.py +++ b/msgspec/__init__.py @@ -16,8 +16,10 @@ ) -def field(*, default=NODEFAULT, default_factory=NODEFAULT, name=None): - return _Field(default=default, default_factory=default_factory, name=name) +def field(*, default=NODEFAULT, default_factory=NODEFAULT, name=None, repr=True): + return _Field( + default=default, default_factory=default_factory, name=name, repr=repr + ) def from_builtins( diff --git a/msgspec/_core.c b/msgspec/_core.c index b426b12b..18b7f041 100644 --- a/msgspec/_core.c +++ b/msgspec/_core.c @@ -2353,6 +2353,7 @@ typedef struct { PyObject *default_value; PyObject *default_factory; PyObject *name; + bool repr; } Field; PyDoc_STRVAR(Field__doc__, @@ -2369,12 +2370,14 @@ PyDoc_STRVAR(Field__doc__, " The name to use when encoding/decoding this field. If present, this\n" " will override any struct-level configuration using the ``rename``\n" " option for this field." +"repr : bool, optional\n" +" Whether to include this field in the generated struct ``__repr__``." ); static PyObject * Field_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) { - char *kwlist[] = {"default", "default_factory", "name", NULL}; + char *kwlist[] = {"default", "default_factory", "name", "repr", NULL}; PyObject *default_value = NODEFAULT, *default_factory = NODEFAULT; - PyObject *name = Py_None; + PyObject *name = Py_None; bool repr = true; if ( !PyArg_ParseTupleAndKeywords( From 2f15b6f993ce4ac8effaa229ba37fe3fa56af0e1 Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Tue, 5 Sep 2023 10:18:57 +0100 Subject: [PATCH 02/11] feat(_core): change repr attribute parsing from bool to int, add missed format specifier, add to `Field_members` and `PyArg_ParseTupleAndKeywords` --- msgspec/_core.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/msgspec/_core.c b/msgspec/_core.c index 18b7f041..85257057 100644 --- a/msgspec/_core.c +++ b/msgspec/_core.c @@ -2377,12 +2377,12 @@ static PyObject * Field_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) { char *kwlist[] = {"default", "default_factory", "name", "repr", NULL}; PyObject *default_value = NODEFAULT, *default_factory = NODEFAULT; - PyObject *name = Py_None; bool repr = true; + PyObject *name = Py_None; int arg_repr = 1; if ( !PyArg_ParseTupleAndKeywords( - args, kwargs, "|$OOO", kwlist, - &default_value, &default_factory, &name + args, kwargs, "|$OOOp", kwlist, + &default_value, &default_factory, &name, &arg_repr ) ) { return NULL; @@ -2416,6 +2416,7 @@ Field_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) { self->default_factory = default_factory; Py_XINCREF(name); self->name = name; + self->repr = arg_repr ? true : false; return (PyObject *)self; } @@ -2458,6 +2459,10 @@ static PyMemberDef Field_members[] = { "name", T_OBJECT, offsetof(Field, name), READONLY, "An alternative name to use when encoding/decoding this field" }, + { + "repr", T_BOOL, offsetof(Field, repr), READONLY, + "Whether to include this field in the generated struct __repr__" + }, {NULL}, }; From f1522e8ffd1ec3e160f85ba2178bd75c7c33dc80 Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Tue, 5 Sep 2023 10:25:26 +0100 Subject: [PATCH 03/11] feat: add `repr` parameter to `field` function signatures in `__init__.pyi` --- msgspec/__init__.pyi | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/msgspec/__init__.pyi b/msgspec/__init__.pyi index 6e4937a0..ab2f2180 100644 --- a/msgspec/__init__.pyi +++ b/msgspec/__init__.pyi @@ -33,11 +33,13 @@ class _NoDefault(enum.Enum): NODEFAULT = _NoDefault.NODEFAULT @overload -def field(*, default: T, name: Optional[str] = None) -> T: ... +def field(*, default: T, name: Optional[str] = None, repr: bool = True) -> T: ... @overload -def field(*, default_factory: Callable[[], T], name: Optional[str] = None) -> T: ... +def field( + *, default_factory: Callable[[], T], name: Optional[str] = None, repr: bool = True +) -> T: ... @overload -def field(*, name: Optional[str] = None) -> Any: ... +def field(*, name: Optional[str] = None, repr: bool = True) -> Any: ... @dataclass_transform(field_specifiers=(field,)) class Struct: __struct_fields__: ClassVar[Tuple[str, ...]] From b5de9333fbfb846b8baed6a58c912cef06ecaeb7 Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Wed, 6 Sep 2023 23:19:31 +0100 Subject: [PATCH 04/11] feat(repr): support configurable `Field` repr --- .gitignore | 2 ++ msgspec/_core.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/.gitignore b/.gitignore index 7c192ef1..898265bf 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,8 @@ ## PyCharm/IntelliJ-generated files *.iml .idea/ +## Vim-generated files +*.sw[op] # Python cached sources __pycache__/ diff --git a/msgspec/_core.c b/msgspec/_core.c index 85257057..e4853065 100644 --- a/msgspec/_core.c +++ b/msgspec/_core.c @@ -2682,6 +2682,7 @@ typedef struct { PyHeapTypeObject base; PyObject *struct_fields; PyObject *struct_defaults; + PyObject *struct_reprs; Py_ssize_t *struct_offsets; PyObject *struct_encode_fields; struct StructInfo *struct_info; @@ -5179,6 +5180,7 @@ rename_mapping(PyObject *rename, PyObject *field) { typedef struct { /* Temporary state. All owned references. */ PyObject *defaults_lk; + PyObject *reprs_lk; PyObject *offsets_lk; PyObject *kwonly_fields; PyObject *slots; @@ -5188,6 +5190,7 @@ typedef struct { PyObject *fields; PyObject *encode_fields; PyObject *defaults; + PyObject *reprs; PyObject *match_args; PyObject *tag; PyObject *tag_field; @@ -5307,6 +5310,7 @@ structmeta_collect_base(StructMetaInfo *info, MsgspecState *mod, PyObject *base) PyObject *fields = st_type->struct_fields; PyObject *encode_fields = st_type->struct_encode_fields; PyObject *defaults = st_type->struct_defaults; + PyObject *reprs = st_type->struct_reprs; Py_ssize_t *offsets = st_type->struct_offsets; Py_ssize_t nfields = PyTuple_GET_SIZE(fields); Py_ssize_t nkwonly = st_type->nkwonly; @@ -5317,10 +5321,12 @@ structmeta_collect_base(StructMetaInfo *info, MsgspecState *mod, PyObject *base) PyObject *field = PyTuple_GET_ITEM(fields, i); PyObject *encode_field = PyTuple_GET_ITEM(encode_fields, i); PyObject *default_val = NODEFAULT; + PyObject *repr_val = PyTuple_GET_ITEM(reprs, i); if (i >= defaults_offset) { default_val = PyTuple_GET_ITEM(defaults, i - defaults_offset); } if (PyDict_SetItem(info->defaults_lk, field, default_val) < 0) return -1; + if (PyDict_SetItem(info->reprs_lk, field, repr_val) < 0) return -1; /* Mark the field as kwonly or not */ if (i >= (nfields - nkwonly)) { @@ -5495,6 +5501,43 @@ structmeta_process_default(StructMetaInfo *info, PyObject *name) { return -1; } +static int +structmeta_process_repr(StructMetaInfo *info, PyObject *name) { + PyObject *obj = PyDict_GetItem(info->namespace, name); + + if (obj == NULL) { + return PyDict_SetItem(info->reprs_lk, name, Py_True); + } + + PyObject* repr_val = NULL; + PyTypeObject *type = Py_TYPE(obj); + + if (type == &Field_Type) { + Field *f = (Field *)obj; + + /* Extract repr boolean or callable */ + if (f->repr == false) { + obj = Py_False; + } + else { + obj = Py_True; + } + repr_val = obj; + Py_INCREF(repr_val); + } + else { + Py_INCREF(obj); + repr_val = Py_True; + } + goto done; + +done: + int status = PyDict_SetItem(info->reprs_lk, name, repr_val); + Py_DECREF(repr_val); + return status; + +} + static int structmeta_is_classvar( StructMetaInfo *info, MsgspecState *mod, PyObject *ann, PyObject **module_ns @@ -5595,6 +5638,7 @@ structmeta_collect_fields(StructMetaInfo *info, MsgspecState *mod, bool kwonly) if (PySet_Discard(info->kwonly_fields, field) < 0) goto error; } + if (structmeta_process_repr(info, field) < 0) goto error; if (structmeta_process_default(info, field) < 0) goto error; } return 0; @@ -5612,6 +5656,7 @@ structmeta_construct_fields(StructMetaInfo *info, MsgspecState *mod) { info->fields = PyTuple_New(nfields); if (info->fields == NULL) return -1; info->defaults = PyList_New(0); + info->reprs = PyList_New(0); /* First pass - handle all non-kwonly fields. */ PyObject *field, *default_val; @@ -5659,12 +5704,27 @@ structmeta_construct_fields(StructMetaInfo *info, MsgspecState *mod) { } } + /* Pass over fields again configuring repr values */ + PyObject *repr_field, *repr_val; + Py_ssize_t repr_field_index = 0; + Py_ssize_t repr_pos = 0; + while (PyDict_Next(info->reprs_lk, &repr_pos, &repr_field, &repr_val)) { + if (PyList_Append(info->reprs, repr_val) < 0) return -1; + repr_field_index++; + } + /* Convert defaults list to tuple */ PyObject *temp_defaults = PyList_AsTuple(info->defaults); Py_DECREF(info->defaults); info->defaults = temp_defaults; if (info->defaults == NULL) return -1; + /* Convert reprs list to tuple */ + PyObject *temp_reprs = PyList_AsTuple(info->reprs); + Py_DECREF(info->reprs); + info->reprs = temp_reprs; + if (info->reprs == NULL) return -1; + /* Compute n_trailing_defaults */ info->nkwonly = nkwonly; info->n_trailing_defaults = 0; @@ -5912,6 +5972,7 @@ StructMeta_new_inner( StructMetaInfo info = { .defaults_lk = NULL, + .reprs_lk = NULL, .offsets_lk = NULL, .kwonly_fields = NULL, .slots = NULL, @@ -5920,6 +5981,7 @@ StructMeta_new_inner( .fields = NULL, .encode_fields = NULL, .defaults = NULL, + .reprs = NULL, .match_args = NULL, .tag = NULL, .tag_field = NULL, @@ -5948,6 +6010,8 @@ StructMeta_new_inner( info.defaults_lk = PyDict_New(); if (info.defaults_lk == NULL) goto cleanup; + info.reprs_lk = PyDict_New(); + if (info.reprs_lk == NULL) goto cleanup; info.offsets_lk = PyDict_New(); if (info.offsets_lk == NULL) goto cleanup; info.kwonly_fields = PySet_New(NULL); @@ -6069,6 +6133,8 @@ StructMeta_new_inner( cls->struct_fields = info.fields; Py_INCREF(info.defaults); cls->struct_defaults = info.defaults; + Py_INCREF(info.reprs); + cls->struct_reprs = info.reprs; Py_INCREF(info.encode_fields); cls->struct_encode_fields = info.encode_fields; Py_INCREF(info.match_args); @@ -6095,6 +6161,7 @@ StructMeta_new_inner( cleanup: /* Temporary structures */ Py_XDECREF(info.defaults_lk); + Py_XDECREF(info.reprs_lk); Py_XDECREF(info.offsets_lk); Py_XDECREF(info.kwonly_fields); Py_XDECREF(info.slots); @@ -6104,6 +6171,7 @@ StructMeta_new_inner( Py_XDECREF(info.fields); Py_XDECREF(info.encode_fields); Py_XDECREF(info.defaults); + Py_XDECREF(info.reprs); Py_XDECREF(info.match_args); Py_XDECREF(info.tag); Py_XDECREF(info.tag_field); @@ -6483,6 +6551,7 @@ StructMeta_traverse(StructMetaObject *self, visitproc visit, void *arg) { Py_VISIT(self->struct_fields); Py_VISIT(self->struct_defaults); + Py_VISIT(self->struct_reprs); Py_VISIT(self->struct_encode_fields); Py_VISIT(self->struct_tag); /* May be a function */ Py_VISIT(self->rename); /* May be a function */ @@ -6499,6 +6568,7 @@ StructMeta_clear(StructMetaObject *self) Py_CLEAR(self->struct_fields); Py_CLEAR(self->struct_defaults); + Py_CLEAR(self->struct_reprs); Py_CLEAR(self->struct_encode_fields); Py_CLEAR(self->struct_tag_field); Py_CLEAR(self->struct_tag_value); @@ -6804,6 +6874,7 @@ StructMeta_config(StructMetaObject *self, void *closure) { static PyMemberDef StructMeta_members[] = { {"__struct_fields__", T_OBJECT_EX, offsetof(StructMetaObject, struct_fields), READONLY, "Struct fields"}, {"__struct_defaults__", T_OBJECT_EX, offsetof(StructMetaObject, struct_defaults), READONLY, "Struct defaults"}, + {"__struct_reprs__", T_OBJECT_EX, offsetof(StructMetaObject, struct_reprs), READONLY, "Struct reprs"}, {"__struct_encode_fields__", T_OBJECT_EX, offsetof(StructMetaObject, struct_encode_fields), READONLY, "Struct encoded field names"}, {"__match_args__", T_OBJECT_EX, offsetof(StructMetaObject, match_args), READONLY, "Positional match args"}, {NULL}, @@ -7075,6 +7146,7 @@ Struct_repr(PyObject *self) { StructMetaObject *st_type = (StructMetaObject *)(Py_TYPE(self)); bool omit_defaults = st_type->repr_omit_defaults == OPT_TRUE; PyObject *fields = st_type->struct_fields; + PyObject *reprs = st_type->struct_reprs; Py_ssize_t nfields = PyTuple_GET_SIZE(fields); PyObject *defaults = NULL; Py_ssize_t nunchecked = nfields; @@ -7100,6 +7172,9 @@ Struct_repr(PyObject *self) { PyObject *val = Struct_get_index(self, i); if (val == NULL) goto error; + PyObject *repr_val = PyTuple_GET_ITEM(reprs, i); + if (repr_val == Py_False) continue; + if (i >= nunchecked) { PyObject *default_val = PyTuple_GET_ITEM(defaults, i - nunchecked); if (is_default(val, default_val)) continue; @@ -7573,6 +7648,13 @@ StructMixin_defaults(PyObject *self, void *closure) { return out; } +static PyObject * +StructMixin_reprs(PyObject *self, void *closure) { + PyObject *out = ((StructMetaObject *)Py_TYPE(self))->struct_reprs; + Py_INCREF(out); + return out; +} + static PyObject* StructMixin_config(StructMetaObject *self, void *closure) { return StructConfig_New((StructMetaObject *)Py_TYPE(self)); @@ -7589,6 +7671,7 @@ static PyGetSetDef StructMixin_getset[] = { {"__struct_fields__", (getter) StructMixin_fields, NULL, "Struct fields", NULL}, {"__struct_encode_fields__", (getter) StructMixin_encode_fields, NULL, "Struct encoded field names", NULL}, {"__struct_defaults__", (getter) StructMixin_defaults, NULL, "Struct defaults", NULL}, + {"__struct_reprs__", (getter) StructMixin_reprs, NULL, "Struct reprs", NULL}, {"__struct_config__", (getter) StructMixin_config, NULL, "Struct configuration", NULL}, {NULL}, }; From 54e5bd31af0cf600728f6d704480875205f432c5 Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Thu, 7 Sep 2023 08:35:34 +0100 Subject: [PATCH 05/11] test: add test coverage for `Field.repr` in `__repr__` and `__rich_repr__` --- tests/test_struct.py | 46 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/test_struct.py b/tests/test_struct.py index 79bb308a..048a0e66 100644 --- a/tests/test_struct.py +++ b/tests/test_struct.py @@ -59,16 +59,19 @@ def test_field(): assert f1.default is NODEFAULT assert f1.default_factory is NODEFAULT assert f1.name is None + assert f1.repr is True f2 = msgspec.field(default=1) assert f2.default == 1 assert f2.default_factory is NODEFAULT assert f2.name is None + assert f2.repr is True f3 = msgspec.field(default_factory=int) assert f3.default is NODEFAULT assert f3.default_factory is int assert f3.name is None + assert f3.repr is True f4 = msgspec.field(name="foo") assert f4.name == "foo" @@ -76,6 +79,9 @@ def test_field(): f5 = msgspec.field(name=None) assert f5.name is None + f6 = msgspec.field(repr=False) + assert f6.repr is False + with pytest.raises(TypeError, match="Cannot set both"): msgspec.field(default=1, default_factory=int) @@ -88,6 +94,7 @@ def test_field(): def test_struct_class_attributes(): assert Struct.__struct_fields__ == () + assert Struct.__struct_reprs__ == () assert Struct.__struct_encode_fields__ == () assert Struct.__struct_defaults__ == () assert Struct.__match_args__ == () @@ -114,6 +121,7 @@ class Test(Struct): assert x.__struct_encode_fields__ == ("c", "b", "a") assert x.__struct_fields__ is x.__struct_encode_fields__ assert x.__struct_defaults__ == ("hello",) + assert x.__struct_reprs__ == (True, True, True) assert x.__slots__ == ("a", "b", "c") assert isinstance(x.__struct_config__, StructConfig) @@ -313,6 +321,7 @@ class Test(Struct): pass assert Test.__struct_fields__ == () + assert Test.__struct_reprs__ == () assert Test.__struct_defaults__ == () assert Test.__match_args__ == () assert Test.__slots__ == () @@ -323,6 +332,7 @@ class Test(Struct): x: int assert Test.__struct_fields__ == ("y", "x") + assert Test.__struct_reprs__ == (True, True) assert Test.__struct_defaults__ == () assert Test.__match_args__ == ("y", "x") assert Test.__slots__ == ("x", "y") @@ -333,6 +343,7 @@ class Test(Struct): x: float = 2.0 assert Test.__struct_fields__ == ("y", "x") + assert Test.__struct_reprs__ == (True, True) assert Test.__struct_defaults__ == (1, 2.0) assert Test.__match_args__ == ("y", "x") assert Test.__slots__ == ("x", "y") @@ -346,6 +357,7 @@ class Test2(Test): pass assert Test2.__struct_fields__ == ("y", "x") + assert Test2.__struct_reprs__ == (True, True) assert Test2.__struct_defaults__ == () assert Test2.__match_args__ == ("y", "x") assert Test2.__slots__ == () @@ -362,6 +374,7 @@ class Test2(Test): f: float = 4.0 assert Test2.__struct_fields__ == ("c", "b", "d", "a", "e", "f") + assert Test2.__struct_reprs__ == (True, True, True, True, True, True) assert Test2.__struct_defaults__ == (1, 2.0, "3.0", 4.0) assert Test2.__match_args__ == ("c", "b", "d", "a", "e", "f") assert Test2.__slots__ == ("e", "f") @@ -379,6 +392,7 @@ class Test2(Test): e: float = 5.0 # new assert Test2.__struct_fields__ == ("c", "b", "d", "a", "e") + assert Test2.__struct_reprs__ == (True, True, True, True) assert Test2.__struct_defaults__ == (3, 4, 2.0, 5.0) assert Test2.__match_args__ == ("c", "b", "d", "a", "e") assert Test2.__slots__ == ("e",) @@ -395,6 +409,7 @@ class B(A, Mixin): a: float = 2.0 assert B.__struct_fields__ == ("b", "a") + assert B.__struct_reprs__ == (True, True) assert B.__struct_defaults__ == (2.0,) assert B.__match_args__ == ("b", "a") assert B.__slots__ == () @@ -427,6 +442,7 @@ class Test(Struct, kw_only=True): a: int assert Test.__struct_fields__ == ("b", "a") + assert Test.__struct_reprs__ == (True, True) assert Test.__struct_defaults__ == () assert Test.__match_args__ == () assert Test.__slots__ == ("a", "b") @@ -439,6 +455,7 @@ class Test(Struct, kw_only=True): d: int = 1 assert Test.__struct_fields__ == ("b", "a", "c", "d") + assert Test.__struct_reprs__ == (True, True, True, True) assert Test.__struct_defaults__ == (0, NODEFAULT, 1) assert Test.__match_args__ == () assert Test.__slots__ == ("a", "b", "c", "d") @@ -457,11 +474,13 @@ class S2(Base): c: int = 1 assert S1.__struct_fields__ == ("d", "c", "b", "a") + assert S1.__struct_reprs__ == (True, True, True, True) assert S1.__struct_defaults__ == () assert S1.__match_args__ == ("d", "c") assert S1.__slots__ == ("c", "d") assert S2.__struct_fields__ == ("d", "c", "b", "a") + assert S2.__struct_reprs__ == (True, True, True, True) assert S2.__struct_defaults__ == (1, NODEFAULT, NODEFAULT) assert S2.__match_args__ == ("d", "c") assert S2.__slots__ == ("c", "d") @@ -476,6 +495,7 @@ class S1(Base): c: int = 2 assert S1.__struct_fields__ == ("d", "c", "b", "a") + assert S1.__struct_reprs__ == (True, True, True, True) assert S1.__struct_defaults__ == (2, 1, NODEFAULT) assert S1.__match_args__ == ("d", "c") assert S1.__slots__ == ("c", "d") @@ -490,6 +510,7 @@ class S1(Base, kw_only=True): c: int assert S1.__struct_fields__ == ("b", "a", "d", "c") + assert S1.__struct_reprs__ == (True, True, True, True) assert S1.__struct_defaults__ == () assert S1.__match_args__ == ("b", "a") assert S1.__slots__ == ("c", "d") @@ -504,6 +525,7 @@ class S1(Base, kw_only=True): c: int = 1 assert S1.__struct_fields__ == ("b", "a", "d", "c") + assert S1.__struct_reprs__ == (True, True, True, True) assert S1.__struct_defaults__ == (0, NODEFAULT, 1) assert S1.__match_args__ == ("b", "a") assert S1.__slots__ == ("c", "d") @@ -518,6 +540,7 @@ class S1(Base, kw_only=True): c: int = 3 assert S1.__struct_fields__ == ("a", "b", "c") + assert S1.__struct_reprs__ == (True, True, True) assert S1.__struct_defaults__ == (2, NODEFAULT, 3) assert S1.__match_args__ == ("a",) assert S1.__slots__ == ("c",) @@ -532,6 +555,7 @@ class S1(Base): c: int = 3 assert S1.__struct_fields__ == ("b", "c", "a") + assert S1.__struct_reprs__ == (True, True, True, True) assert S1.__struct_defaults__ == (3, 2) assert S1.__match_args__ == ("b", "c") assert S1.__slots__ == ("c",) @@ -740,6 +764,28 @@ class Test(Struct, repr_omit_defaults=True): assert repr(x) == "Test(a=0, b=1, c='two')" assert x.__rich_repr__() == [("a", 0), ("b", 1), ("c", "two")] + def test_repr_custom_false_one_field(self): + class Test(Struct): + a: int = msgspec.field(default=0, repr=False) + + x = Test(0) + y = Test(1) + assert repr(x) == repr(y) == "Test()" + assert x.__rich_repr__() == y.__rich_repr__() == [] + + def test_repr_custom_false_one_of_multiple_fields(self): + class Test(Struct): + a: int + b: int = msgspec.field(default=0, repr=False) + + x = Test(0) + assert repr(x) == "Test(a=0)" + assert x.__rich_repr__() == [("a", 0)] + + x = Test(0, b=1) + assert repr(x) == "Test(a=0)" + assert x.__rich_repr__() == [("a", 0)] + def test_repr_recursive(self): class Test(Struct): a: int From b2bc6fc4da17ce96a8f21bbd02a584900e2e97b2 Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Thu, 7 Sep 2023 08:51:26 +0100 Subject: [PATCH 06/11] feat(rich repr): skip `repr=False` fields from the `__rich_repr__` as well; fix test nits --- msgspec/_core.c | 7 ++++++- tests/test_struct.py | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/msgspec/_core.c b/msgspec/_core.c index e4853065..c88cd812 100644 --- a/msgspec/_core.c +++ b/msgspec/_core.c @@ -7173,7 +7173,7 @@ Struct_repr(PyObject *self) { if (val == NULL) goto error; PyObject *repr_val = PyTuple_GET_ITEM(reprs, i); - if (repr_val == Py_False) continue; + if (repr_val == Py_False) continue; if (i >= nunchecked) { PyObject *default_val = PyTuple_GET_ITEM(defaults, i - nunchecked); @@ -7592,6 +7592,7 @@ Struct_rich_repr(PyObject *self, PyObject *args) { StructMetaObject *st_type = (StructMetaObject *)(Py_TYPE(self)); bool omit_defaults = st_type->repr_omit_defaults == OPT_TRUE; PyObject *fields = st_type->struct_fields; + PyObject *reprs = st_type->struct_reprs; Py_ssize_t nfields = PyTuple_GET_SIZE(fields); PyObject *defaults = NULL; Py_ssize_t nunchecked = nfields; @@ -7608,12 +7609,16 @@ Struct_rich_repr(PyObject *self, PyObject *args) { PyObject *field = PyTuple_GET_ITEM(fields, i); PyObject *val = Struct_get_index(self, i); + PyObject *repr_val = PyTuple_GET_ITEM(reprs, i); + if (repr_val == Py_False) continue; + if (i >= nunchecked) { PyObject *default_val = PyTuple_GET_ITEM(defaults, i - nunchecked); if (is_default(val, default_val)) continue; } if (val == NULL) goto error; + PyObject *part = PyTuple_Pack(2, field, val); if (part == NULL) goto error; int status = PyList_Append(out, part); diff --git a/tests/test_struct.py b/tests/test_struct.py index 048a0e66..183927df 100644 --- a/tests/test_struct.py +++ b/tests/test_struct.py @@ -392,7 +392,7 @@ class Test2(Test): e: float = 5.0 # new assert Test2.__struct_fields__ == ("c", "b", "d", "a", "e") - assert Test2.__struct_reprs__ == (True, True, True, True) + assert Test2.__struct_reprs__ == (True, True, True, True, True) assert Test2.__struct_defaults__ == (3, 4, 2.0, 5.0) assert Test2.__match_args__ == ("c", "b", "d", "a", "e") assert Test2.__slots__ == ("e",) @@ -555,7 +555,7 @@ class S1(Base): c: int = 3 assert S1.__struct_fields__ == ("b", "c", "a") - assert S1.__struct_reprs__ == (True, True, True, True) + assert S1.__struct_reprs__ == (True, True, True) assert S1.__struct_defaults__ == (3, 2) assert S1.__match_args__ == ("b", "c") assert S1.__slots__ == ("c",) From 267ab27aa4ffe46d81c2ea33e276d0be844ba1a3 Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Thu, 7 Sep 2023 22:17:38 +0100 Subject: [PATCH 07/11] test: coverage for callable repr feature --- tests/test_struct.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/tests/test_struct.py b/tests/test_struct.py index 183927df..66fc9d31 100644 --- a/tests/test_struct.py +++ b/tests/test_struct.py @@ -10,13 +10,13 @@ from inspect import Parameter, Signature from typing import Any, List, Optional -import pytest -from utils import temp_module - import msgspec +import pytest from msgspec import NODEFAULT, UNSET, Struct, defstruct, field from msgspec.structs import StructConfig, replace +from utils import temp_module + @contextmanager def nogc(): @@ -82,6 +82,9 @@ def test_field(): f6 = msgspec.field(repr=False) assert f6.repr is False + f7 = msgspec.field(repr=hex) + assert f7.repr is hex + with pytest.raises(TypeError, match="Cannot set both"): msgspec.field(default=1, default_factory=int) @@ -786,6 +789,16 @@ class Test(Struct): assert repr(x) == "Test(a=0)" assert x.__rich_repr__() == [("a", 0)] + def test_repr_custom_callable_and_false(self): + class Test(Struct): + a: int + b: int = msgspec.field(repr=hex) + c: int = msgspec.field(repr=False) + + x = Test(a=1, b=2, c=3) + assert repr(x) == "Test(a=1, b=0x2)" + assert x.__rich_repr__() == [("a", 1), ("b", "0x2")] + def test_repr_recursive(self): class Test(Struct): a: int From 775bed88c988da0c4bf5db8d57fc002a173fee15 Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Fri, 8 Sep 2023 00:06:24 +0100 Subject: [PATCH 08/11] feat: support for callable Field repr --- msgspec/_core.c | 66 ++++++++++++++++++++++++++++---------------- tests/test_struct.py | 2 +- 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/msgspec/_core.c b/msgspec/_core.c index c88cd812..ac70c377 100644 --- a/msgspec/_core.c +++ b/msgspec/_core.c @@ -2353,7 +2353,7 @@ typedef struct { PyObject *default_value; PyObject *default_factory; PyObject *name; - bool repr; + PyObject *repr; } Field; PyDoc_STRVAR(Field__doc__, @@ -2370,19 +2370,19 @@ PyDoc_STRVAR(Field__doc__, " The name to use when encoding/decoding this field. If present, this\n" " will override any struct-level configuration using the ``rename``\n" " option for this field." -"repr : bool, optional\n" +"repr : bool or Callable, optional\n" " Whether to include this field in the generated struct ``__repr__``." ); static PyObject * Field_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) { char *kwlist[] = {"default", "default_factory", "name", "repr", NULL}; PyObject *default_value = NODEFAULT, *default_factory = NODEFAULT; - PyObject *name = Py_None; int arg_repr = 1; + PyObject *name = Py_None; PyObject *repr = Py_True; if ( !PyArg_ParseTupleAndKeywords( - args, kwargs, "|$OOOp", kwlist, - &default_value, &default_factory, &name, &arg_repr + args, kwargs, "|$OOOO", kwlist, + &default_value, &default_factory, &name, &repr ) ) { return NULL; @@ -2393,6 +2393,12 @@ Field_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) { ); return NULL; } + if (!PyBool_Check(repr)) { + if (!PyCallable_Check(repr)) { + PyErr_SetString(PyExc_TypeError, "repr must be boolean or callable"); + return NULL; + } + } if (default_factory != NODEFAULT) { if (!PyCallable_Check(default_factory)) { PyErr_SetString(PyExc_TypeError, "default_factory must be callable"); @@ -2416,7 +2422,8 @@ Field_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) { self->default_factory = default_factory; Py_XINCREF(name); self->name = name; - self->repr = arg_repr ? true : false; + Py_XINCREF(repr); + self->repr = repr; return (PyObject *)self; } @@ -2426,6 +2433,7 @@ Field_traverse(Field *self, visitproc visit, void *arg) Py_VISIT(self->default_value); Py_VISIT(self->default_factory); Py_VISIT(self->name); + Py_VISIT(self->repr); return 0; } @@ -2435,6 +2443,7 @@ Field_clear(Field *self) Py_CLEAR(self->default_value); Py_CLEAR(self->default_factory); Py_CLEAR(self->name); + Py_CLEAR(self->repr); return 0; } @@ -5514,28 +5523,18 @@ structmeta_process_repr(StructMetaInfo *info, PyObject *name) { if (type == &Field_Type) { Field *f = (Field *)obj; - /* Extract repr boolean or callable */ - if (f->repr == false) { - obj = Py_False; - } - else { - obj = Py_True; - } - repr_val = obj; + repr_val = f->repr; Py_INCREF(repr_val); } else { - Py_INCREF(obj); repr_val = Py_True; + Py_INCREF(repr_val); } - goto done; -done: int status = PyDict_SetItem(info->reprs_lk, name, repr_val); Py_DECREF(repr_val); return status; - } static int @@ -7172,8 +7171,8 @@ Struct_repr(PyObject *self) { PyObject *val = Struct_get_index(self, i); if (val == NULL) goto error; - PyObject *repr_val = PyTuple_GET_ITEM(reprs, i); - if (repr_val == Py_False) continue; + PyObject *repr_config = PyTuple_GET_ITEM(reprs, i); + if (repr_config == Py_False) continue; if (i >= nunchecked) { PyObject *default_val = PyTuple_GET_ITEM(defaults, i - nunchecked); @@ -7190,7 +7189,16 @@ Struct_repr(PyObject *self) { if (!strbuilder_extend_unicode(&builder, field)) goto error; if (!strbuilder_extend_literal(&builder, "=")) goto error; - PyObject *repr = PyObject_Repr(val); + PyObject *repr; + if (PyCallable_Check(repr_config)) { + PyObject *repr_input; + repr_input = PyObject_CallFunctionObjArgs(repr_config, val, NULL); + if (repr_input == NULL) goto error; + repr = PyObject_Repr(repr_input); + Py_DECREF(repr_input); + } else { + repr = PyObject_Repr(val); + } if (repr == NULL) goto error; bool ok = strbuilder_extend_unicode(&builder, repr); Py_DECREF(repr); @@ -7609,8 +7617,8 @@ Struct_rich_repr(PyObject *self, PyObject *args) { PyObject *field = PyTuple_GET_ITEM(fields, i); PyObject *val = Struct_get_index(self, i); - PyObject *repr_val = PyTuple_GET_ITEM(reprs, i); - if (repr_val == Py_False) continue; + PyObject *repr_config = PyTuple_GET_ITEM(reprs, i); + if (repr_config == Py_False) continue; if (i >= nunchecked) { PyObject *default_val = PyTuple_GET_ITEM(defaults, i - nunchecked); @@ -7619,8 +7627,18 @@ Struct_rich_repr(PyObject *self, PyObject *args) { if (val == NULL) goto error; - PyObject *part = PyTuple_Pack(2, field, val); + PyObject *packing_val; + if (PyCallable_Check(repr_config)) { + packing_val = PyObject_CallFunctionObjArgs(repr_config, val, NULL); + Py_DECREF(val); + if (packing_val == NULL) goto error; + } else { + packing_val = val; + } + + PyObject *part = PyTuple_Pack(2, field, packing_val); if (part == NULL) goto error; + int status = PyList_Append(out, part); Py_DECREF(part); if (status < 0) goto error; diff --git a/tests/test_struct.py b/tests/test_struct.py index 66fc9d31..7b76ed49 100644 --- a/tests/test_struct.py +++ b/tests/test_struct.py @@ -796,7 +796,7 @@ class Test(Struct): c: int = msgspec.field(repr=False) x = Test(a=1, b=2, c=3) - assert repr(x) == "Test(a=1, b=0x2)" + assert repr(x) == "Test(a=1, b='0x2')" assert x.__rich_repr__() == [("a", 1), ("b", "0x2")] def test_repr_recursive(self): From d109a19d01b5eba2714dfbca040500448f653acb Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Fri, 8 Sep 2023 00:15:49 +0100 Subject: [PATCH 09/11] fix(attribute type): all tests pass --- msgspec/_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/msgspec/_core.c b/msgspec/_core.c index ac70c377..c153e1c2 100644 --- a/msgspec/_core.c +++ b/msgspec/_core.c @@ -2469,7 +2469,7 @@ static PyMemberDef Field_members[] = { "An alternative name to use when encoding/decoding this field" }, { - "repr", T_BOOL, offsetof(Field, repr), READONLY, + "repr", T_OBJECT_EX, offsetof(Field, repr), READONLY, "Whether to include this field in the generated struct __repr__" }, {NULL}, From 7e68029aa6a5deb9a73e0a6560170d315194802a Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Fri, 8 Sep 2023 01:03:33 +0100 Subject: [PATCH 10/11] fix: improve memory management --- msgspec/_core.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/msgspec/_core.c b/msgspec/_core.c index c153e1c2..bd8d6094 100644 --- a/msgspec/_core.c +++ b/msgspec/_core.c @@ -7168,15 +7168,26 @@ Struct_repr(PyObject *self) { for (Py_ssize_t i = 0; i < nfields; i++) { PyObject *field = PyTuple_GET_ITEM(fields, i); + Py_INCREF(field); + PyObject *val = Struct_get_index(self, i); if (val == NULL) goto error; PyObject *repr_config = PyTuple_GET_ITEM(reprs, i); - if (repr_config == Py_False) continue; + Py_INCREF(repr_config); + if (repr_config == Py_False) { + Py_DECREF(repr_config); + continue; + } if (i >= nunchecked) { PyObject *default_val = PyTuple_GET_ITEM(defaults, i - nunchecked); - if (is_default(val, default_val)) continue; + Py_INCREF(default_val); + + if (is_default(val, default_val)) { + Py_DECREF(default_val); + continue; + } } if (first) { @@ -7199,6 +7210,8 @@ Struct_repr(PyObject *self) { } else { repr = PyObject_Repr(val); } + Py_DECREF(field); + Py_DECREF(repr_config); if (repr == NULL) goto error; bool ok = strbuilder_extend_unicode(&builder, repr); Py_DECREF(repr); From 78ae7e69c9aedad475e62e2f1e749a12cc393d39 Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Fri, 8 Sep 2023 09:38:07 +0100 Subject: [PATCH 11/11] test: cover TypeErrors from `Field.repr` with non-bool/callable arguments --- tests/test_struct.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_struct.py b/tests/test_struct.py index 7b76ed49..bfc1d427 100644 --- a/tests/test_struct.py +++ b/tests/test_struct.py @@ -94,6 +94,10 @@ def test_field(): with pytest.raises(TypeError, match="must be a str or None"): msgspec.field(name=b"bad") + for repr_value in ("a", b"b", 1, 2.3, None): + with pytest.raises(TypeError, match="repr must be boolean or callable"): + msgspec.field(repr=repr_value) + def test_struct_class_attributes(): assert Struct.__struct_fields__ == ()