Repository: python/cpython
Pull request: gh-107406: Add better `struct.Struct` repr
fixes #107406

<!-- gh-issue-number: gh-107406 -->
* Issue: gh-107406
<!-- /gh-issue-number -->


Problem statement:
better `struct.Struct` repr
# Feature or enhancement
Add better `struct.Struct` repr

# Pitch
```py
>>> import struct
>>> struct.Struct('i')
<_struct.Struct at 0x00223FC2C7290>
```
Hmm, what is the format of this struct? It is not clear from repr.

Something like this would be better:
```py
>>> import struct
>>> struct.Struct('i')
Struct('i')
```


<!-- gh-linked-prs -->
### Linked PRs
* gh-107407
<!-- /gh-linked-prs -->

Hints from issue discussion:

Commit message: add better `struct.Struct` repr

Patch to review:
diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py
index 6b1f22f66fd157..c76649cdcd9cce 100644
--- a/Lib/test/test_struct.py
+++ b/Lib/test/test_struct.py
@@ -774,6 +774,10 @@ def test_error_propagation(fmt_str):
         test_error_propagation('N')
         test_error_propagation('n')
 
+    def test_repr(self):
+        s = struct.Struct('=i2H')
+        self.assertEqual(repr(s), f'Struct({s.format!r})')
+
 class UnpackIteratorTest(unittest.TestCase):
     """
     Tests for iterative unpacking (struct.Struct.iter_unpack).
diff --git a/Misc/NEWS.d/next/Library/2023-07-29-02-01-24.gh-issue-107406.ze6sQP.rst b/Misc/NEWS.d/next/Library/2023-07-29-02-01-24.gh-issue-107406.ze6sQP.rst
new file mode 100644
index 00000000000000..fa14c84543ec86
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2023-07-29-02-01-24.gh-issue-107406.ze6sQP.rst
@@ -0,0 +1 @@
+Add better `struct.Struct.__repr__` implementation.
diff --git a/Modules/_struct.c b/Modules/_struct.c
index 425715ad030d4d..b78f83440b8178 100644
--- a/Modules/_struct.c
+++ b/Modules/_struct.c
@@ -2164,6 +2164,19 @@ s_sizeof(PyStructObject *self, void *unused)
     return PyLong_FromSize_t(size);
 }
 
+static PyObject *
+s_repr(PyStructObject *self)
+{
+    PyObject* fmt;
+    PyObject* s;
+
+    fmt = PyUnicode_FromStringAndSize(PyBytes_AS_STRING(self->s_format),
+                                      PyBytes_GET_SIZE(self->s_format));
+    s = PyUnicode_FromFormat("%s(%R)", _PyType_Name(Py_TYPE(self)), fmt);
+    Py_DECREF(fmt);
+    return s;
+}
+
 /* List of functions */
 
 static struct PyMethodDef s_methods[] = {
@@ -2196,6 +2209,7 @@ static PyType_Slot PyStructType_slots[] = {
     {Py_tp_dealloc, s_dealloc},
     {Py_tp_getattro, PyObject_GenericGetAttr},
     {Py_tp_setattro, PyObject_GenericSetAttr},
+    {Py_tp_repr, s_repr},
     {Py_tp_doc, (void*)s__doc__},
     {Py_tp_traverse, s_traverse},
     {Py_tp_clear, s_clear},


Code review comments:
diff_hunk: @@ -2164,6 +2164,19 @@ s_sizeof(PyStructObject *self, void *unused)
     return PyLong_FromSize_t(size);
 }
 
+static PyObject *
+s_repr(PyStructObject *self)
+{
+    PyObject* fmt;
path: Modules/_struct.c
text: @user1:
You don't need to declare all variables at the top.

diff_hunk: @@ -2164,6 +2164,19 @@ s_sizeof(PyStructObject *self, void *unused)
     return PyLong_FromSize_t(size);
 }
 
+static PyObject *
+s_repr(PyStructObject *self)
+{
+    PyObject* fmt;
+    PyObject* s;
+
+    fmt = PyUnicode_FromStringAndSize(PyBytes_AS_STRING(self->s_format),
path: Modules/_struct.c
text: @user1:
Missing a check for this function returning NULL.

Also, this feels somewhat roundabout, wonder if there's a way to do this without going through an intermediate Unicode object.

@author:
`s_get_format` function which implements `s.format` getter is doing the same thing without error checking.
Should I fix them both or leave it as is?

@user1:
That one is fine as it immediately returns the value from `PyUnicode_FromStringAndSize`, so the exception will get propagated.