Skip to content

bpo-21071: struct.Struct.format type is now str #845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vstinner merged 1 commit into from
Jun 23, 2017
Merged

bpo-21071: struct.Struct.format type is now str #845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Doc/library/struct.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,9 @@ The :mod:`struct` module also defines the following type:

The format string used to construct this Struct object.

.. versionchanged:: 3.7
The format string type is now :class:`str` instead of :class:`bytes`.

.. attribute:: size

The calculated size of the struct (and hence of the bytes object produced
Expand Down
3 changes: 3 additions & 0 deletions Doc/whatsnew/3.7.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,9 @@ Changes in the Python API
``makedirs()``.
(Contributed by Serhiy Storchaka in :issue:`19930`.)

* The :attr:`struct.Struct.format` type is now :class:`str` instead of
:class:`bytes`. (Contributed by Victor Stinner in :issue:`21071`.)


CPython bytecode changes
------------------------
Expand Down
8 changes: 8 additions & 0 deletions Lib/test/test_struct.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,14 @@ def test_issue29802(self):
# Shouldn't crash.
self.assertEqual(struct.unpack(b'b', b'a'), (b'a'[0],))

def test_format_attr(self):
s = struct.Struct('=i2H')
self.assertEqual(s.format, '=i2H')
Copy link
Member

@berkerpeksag berkerpeksag Jun 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, self.assertIsInstance(s.format, str) would be better to understand (for future readers) the behavior change.


# use a bytes string
s2 = struct.Struct(s.format.encode())
self.assertEqual(s2.format, s.format)


class UnpackIteratorTest(unittest.TestCase):
"""
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,9 @@ Extension Modules
Library
-------

- bpo-21071: struct.Struct.format type is now :class:`str` instead of
Copy link
Member

@serhiy-storchaka serhiy-storchaka Jun 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think :class: is not needed.

:class:`bytes`.

- bpo-29212: Fix concurrent.futures.thread.ThreadPoolExecutor threads to have
a non repr() based thread name by default when no thread_name_prefix is
supplied. They will now identify themselves as "ThreadPoolExecutor-y_n".
Expand Down
4 changes: 2 additions & 2 deletions Modules/_struct.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1957,8 +1957,8 @@ s_pack_into(PyObject *self, PyObject **args, Py_ssize_t nargs, PyObject *kwnames
static PyObject *
s_get_format(PyStructObject *self, void *unused)
{
Py_INCREF(self->s_format);
return self->s_format;
return PyUnicode_FromStringAndSize(PyBytes_AS_STRING(self->s_format),
Copy link
Member

@serhiy-storchaka serhiy-storchaka Jun 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make s_format an instance of str?

Copy link
Member Author

@vstinner vstinner Jun 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to write minimum changes. I expect that most of the code is written to work with C strings char*, not with Python Unicode strings.

Copy link
Member

@vadmium vadmium Jun 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I already did this in my format-str.patch. I was able to get a char* out of the string with PyUnicode_AsUTF8.

PyBytes_GET_SIZE(self->s_format));
}

static PyObject *
Expand Down