GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-143715: Deprecate incomplete initialization of struct.Struct() by serhiy-storchaka · Pull Request #145580 · python/cpython

Conversation

serhiy-storchaka commented Mar 6, 2026 •

edited by github-actions Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

Struct.__new__() will require a mandatory argument (format)
Calls of __init__() method on initialized Struct are deprecated

This is an evolution of #143659, but since virtually all code and test were rewritten I created a new PR.

Issue: Make struct.Struct() really immutable #143715

📚 Documentation preview 📚: https://cpython-previews--145580.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

skirpichev and others added 29 commits

January 10, 2026 16:37


          pythongh-78724: deprecate incomplete initialization of struct.Struct()

fff147f

* ``Struct.__new__()`` will require a mandatory argument (format)
* Calls of ``__init__()`` method on initialized Struct are deprecated


          Merge branch 'master' into deprecate-struct-init-2/78724

d2a0345


          address review: test_Struct_reinitialization()

589fbc7


          catch new warning in test_operations_on_half_initialized_Struct()

5e91e87


          add init_called flag

babb274


          Merge branch 'master' into deprecate-struct-init-2/78724

0e4e84b


          a hack to support new idiom for subclassing

3f36635

This make format argument in the __init__() - optional.  If it's
missing, the object must be already initialized in __new__().


          Merge branch 'master' into deprecate-struct-init-2/78724

e576475


          + filter out Struct signature test

628aadd


          Update Modules/_struct.c

979cc18

Co-authored-by: Victor Stinner <[email protected]>


          Merge branch 'master' into deprecate-struct-init-2/78724

f7492ec


          address review: reformat s_new()

db8f5f2


          address review: actual___init___impl -> s_init

dc8cbef


          Merge branch 'master' into deprecate-struct-init-2/78724

6206ae1


          Update Doc/deprecations/pending-removal-in-3.20.rst

12143d1

Co-authored-by: Serhiy Storchaka <[email protected]>


          Merge branch 'master' into deprecate-struct-init-2/78724

d4ca6b8


          + rename news

6b9b4fb


          Apply suggestions from code review

79961a2


          address review: add test

58550c0


          address review: fix for format

c815882


          address review: reformat if blocks

f1388ee


          address review: s_new

685a6c4


          Merge branch 'master' into deprecate-struct-init-2/78724

76f8723


          address review: add fake signature for Struct

538d301


          +test new idiom

09d6ebd


          Some cleanup; also add reference to issue and reverted PR

7f0a133


          Merge branch 'master' into deprecate-struct-init-2/78724

26cde89


          issue DeprecationWarning for implicit tp_new calls in subclasses

This catch current pattern for Struct's subclassing like

class MyStruct(Struct):
    def __init__(self):
        super().__init__('>h')


          Fix more corner cases.

05eb292

serhiy-storchaka requested a review from vstinner

March 6, 2026 09:01

serhiy-storchaka requested a review from skirpichev

March 6, 2026 09:01

serhiy-storchaka requested a review from AA-Turner as a code owner

March 6, 2026 09:01

bedevere-app Bot mentioned this pull request

Mar 6, 2026

Make struct.Struct() really immutable #143715

Closed

bedevere-app Bot added the awaiting core review label

Mar 6, 2026

serhiy-storchaka mentioned this pull request

Mar 6, 2026

gh-143715: deprecate incomplete initialization of struct.Struct() #143659

Closed

skirpichev reviewed

Mar 7, 2026

View reviewed changes

Comment thread

Doc/deprecations/pending-removal-in-3.20.rst Outdated

		:meth:`~object.__init__` method on initialized :class:`~struct.Struct`
		objects is deprecated and will be removed in Python 3.20.

		(Contributed by Sergey B Kirpichev in :gh:`143715`.)

skirpichev Mar 7, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change

	(Contributed by Sergey B Kirpichev in :gh:`143715`.)
	(Contributed by Serhiy Storchaka in :gh:`143715`.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Doc/whatsnew/3.15.rst Outdated

		:meth:`~object.__init__` method on initialized :class:`~struct.Struct`
		objects is deprecated and will be removed in Python 3.20.

		(Contributed by Sergey B Kirpichev in :gh:`143715`.)

skirpichev Mar 7, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change

	(Contributed by Sergey B Kirpichev in :gh:`143715`.)
	(Contributed by Serhiy Storchaka in :gh:`143715`.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_struct.py Outdated

		def check_sizeof(self, format_str, number_of_codes):
		# The size of 'PyStructObject'
		totalsize = support.calcobjsize('2n3P')
		totalsize = support.calcobjsize('2n3P1?')

skirpichev Mar 7, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change

	totalsize = support.calcobjsize('2n3P1?')
	totalsize = support.calcobjsize('2n3P?')

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

serhiy-storchaka Mar 7, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It should be '2n3P?0P', with padding.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_struct.py

		super().__init__('>h')

		my_struct = MyStruct('>h')
		self.assertEqual(my_struct.pack(12345), b'\x30\x39')

skirpichev Mar 7, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think this will be more clear, per Victor's suggestion:

Suggested change

	self.assertEqual(my_struct.pack(12345), b'\x30\x39')
	self.assertEqual(my_struct.format, '>h')

(and in all cases below too)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

serhiy-storchaka Mar 7, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

What if self->s_format and and self->s_codes are de-synchronized? The original test used Struct.pack().

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

skirpichev Mar 8, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The original test used Struct.pack().

That's true. But I think that with format field tests will be more clear.

What if self->s_format and and self->s_codes are de-synchronized?

I see, you test some such cases with bad characters.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

serhiy-storchaka Mar 9, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I added assertions for format, but we need also functional tests, because internals can be initialized twice, and if something goes wrong, we will and with inconsistent format and pack(). See new cases in test_Struct_reinitialization. We should fix this in other issue.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_struct.py

		def __init__(self, args, *kwargs):
		super().__init__('>h')

		my_struct = MyStruct('>h')

skirpichev Mar 7, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Why this not raises a warning? I think we should warn in all cases, where Struct.__init__() was explicitly called. // #143659 (comment)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

serhiy-storchaka Mar 7, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Because this works with old and with new code. Both __new__ and __init__ take the same argument.

The code

class MyStruct(struct.Struct):
    def __init__(self, format):
        super().__init__(format)
        # some other initialization

works now and will work in future.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

encukou reacted with thumbs up emoji

All reactions

👍 1 reaction

Comment thread

Lib/test/test_struct.py


		my_struct = MyStruct('>h')
		self.assertEqual(my_struct.pack(12345), b'\x30\x39')
		my_struct = MyStruct(format='>h')

skirpichev Mar 7, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ah, I entirely forgot that format is a positional or keyword argument in the master. I'll fix my patch.

But again, this should emit a warning.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_struct.py

Comment on lines +880 to +888

		# New way, no warnings:
		class MyStruct(struct.Struct):
		def __new__(cls, newargs, initargs):
		return super().__new__(cls, *newargs)
		def __init__(self, newargs, initargs):
		if initargs is not None:
		super().__init__(*initargs)

		my_struct = MyStruct(('>h',), ('>h',))

skirpichev Mar 7, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Again, why no warnings here?

BTW, I doubt this usage pattern come from reality ;-)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

serhiy-storchaka Mar 7, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Because __new__() and __init__() are called with the same argument, as expected. You are supposed to do this if you write a code that works in all versions.

As for other cases, having both custom __new__() and __init__() which call corresponding parent's methods with different arguments is unusual, but this need to be tested. I found bugs in my code when added these tests.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

encukou reacted with heart emoji

All reactions

❤️ 1 reaction

serhiy-storchaka added 2 commits

March 7, 2026 14:05


          Update credicts.

d9c0488


          Fix format for the __sizeof__ test.

3de045b

skirpichev self-requested a review

March 8, 2026 00:10

skirpichev reviewed

Mar 9, 2026

View reviewed changes

skirpichev left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This looks ok for me and have much better test coverage than my pr #143659.

Though, I worry that it's too complex: #143659 (comment)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_struct.py

		super().__init__('>h')

		my_struct = MyStruct('>h')
		self.assertEqual(my_struct.pack(12345), b'\x30\x39')

skirpichev Mar 8, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The original test used Struct.pack().

That's true. But I think that with format field tests will be more clear.

What if self->s_format and and self->s_codes are de-synchronized?

I see, you test some such cases with bad characters.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

serhiy-storchaka added 2 commits

March 9, 2026 11:23


          Fix test_Struct_reinitialization on big-endian and add tests for inva…

ebcf358

…lid format.


          Add assertions for format.

e003dab

serhiy-storchaka commented

Mar 9, 2026

View reviewed changes

serhiy-storchaka left a comment

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The code is complex because the problem is complex.

We should minimize user inconvenience. If the code works in past and future versions, it should not emit warnings. There should be a way to write a warning-free code compatible with old versions (because you usually support more than one Python versions), and it should be valid in future versions.

If ignore this, we could just make a breaking change without deprecation period. And we do this if there is no other way (for example, the format attribute is now a string, not a bytes object like in older versions -- it was impossible to make this change not abrupt).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_struct.py

		super().__init__('>h')

		my_struct = MyStruct('>h')
		self.assertEqual(my_struct.pack(12345), b'\x30\x39')

serhiy-storchaka Mar 9, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

skirpichev commented Mar 10, 2026

Copy link

Copy Markdown

Member

There should be a way to write a warning-free code compatible with old versions (because you usually support more than one Python versions), and it should be valid in future versions.

As I said in #143659 (comment), this requirement looks too strong for me. To support several versions, people could also use if sys.version_info blocks.

CC @encukou, would you mind to review this pr?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner reviewed

Mar 10, 2026

View reviewed changes

Comment thread

Modules/_struct.c Outdated

		return -1;
		}
		Py_SETREF(self->s_format, format);
		if (prepare_s(self)) {

vstinner Mar 10, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

What do you think of restoring s_format to its previous value if prepare_s() fails?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_struct.py Outdated

		def __new__(cls, args, *kwargs):
		return super().__new__(cls, '>h')

		my_struct = MyStruct('>h')

vstinner Mar 10, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

You can factorize some tests using ony 1 positional argument:

        for format in ('>h', '<h', 42, '$', '\u20ac'):
            with self.subTest(format=format):
                my_struct = MyStruct(format)
                self.assertEqual(my_struct.format, '>h')
                self.assertEqual(my_struct.pack(12345), b'\x30\x39')

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_struct.py Outdated

		with self.assertWarns(DeprecationWarning):
		with self.assertRaises(struct.error):
		s.__init__('$')
		self.assertEqual(s.format, '$')

vstinner Mar 10, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I would prefer to restore the old format if __init__() tries to set an invalid format string.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

serhiy-storchaka Mar 10, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This is a different issue. See #145744.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner reacted with thumbs up emoji

All reactions

👍 1 reaction

serhiy-storchaka added 2 commits

March 10, 2026 13:49


          Factorize some tests.

d453990


          Merge branch 'main' into deprecate-struct-init

66dbc04

encukou approved these changes

Mar 11, 2026

View reviewed changes

encukou left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I like this approach! Thank you for caring about backwards compatibility!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Doc/deprecations/pending-removal-in-3.20.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot added awaiting merge and removed awaiting core review labels

Mar 11, 2026

serhiy-storchaka and others added 3 commits

March 11, 2026 16:38


          Update Doc/deprecations/pending-removal-in-3.20.rst

6d60b16

Co-authored-by: Petr Viktorin <[email protected]>


          Merge branch 'main' into deprecate-struct-init

d7da0f8


          Emit FutureWarning instead of DeprecationWarning for re-initialization.

ca960a8

serhiy-storchaka commented Mar 11, 2026

Copy link

Copy Markdown

Member Author

About deprecation warning in the repeated __init__(). I suggested this as a simple intermediate step, but if in future it will be identical to object.__init__(), which is no-op, there should not be warning for calling it with the same format (which is also no-op currently), but calling it with different format should emit a FutureWarning instead of a DeprecationWarning.

Accidentally, this also makes the code a little bit simpler.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka merged commit 7245630 into python:main

Mar 12, 2026

91 of 92 checks passed

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the awaiting merge label

Mar 12, 2026

serhiy-storchaka deleted the deprecate-struct-init branch

March 12, 2026 07:44

ljfp pushed a commit to ljfp/cpython that referenced this pull request

Apr 25, 2026


          pythongh-143715: Deprecate incomplete initialization of struct.Struct…

914fe07

…() (pythonGH-145580)

* Struct.__new__() will require a mandatory argument (format)
* Calls of __init__() method with a different format argument on initialized
  Struct are deprecated

Co-authored-by: Sergey B Kirpichev <[email protected]>

Reviewers

vstinner

vstinner left review comments

skirpichev

skirpichev left review comments

encukou

encukou approved these changes

AA-Turner

Awaiting requested review from AA-Turner

AA-Turner is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

serhiy-storchaka commented Mar 6, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skirpichev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skirpichev commented Mar 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

encukou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

serhiy-storchaka commented Mar 11, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

serhiy-storchaka commented Mar 6, 2026 •

edited by github-actions Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.