FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-143715: deprecate incomplete initialization of struct.Struct() by skirpichev · Pull Request #143659 · python/cpython · GitHub

Repository navigation

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

Closed
skirpichev wants to merge 33 commits into
python:mainfrom
skirpichev:deprecate-struct-init-2/78724
Closed

skirpichev wants to merge 33 commits into
python:mainfrom
skirpichev:deprecate-struct-init-2/78724

Conversation

skirpichev commented Jan 10, 2026 •
edited by serhiy-storchaka
Loading

Copy link
Copy Markdown
Member
  • Struct.__new__() will require a mandatory argument (format)
  • Calls of __init__() method on initialized Struct are deprecated

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

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

Copy link
Copy Markdown
Member Author

The evil plan is to remove custom Struct.__init__() method and move all initialization logic to the Struct.__new__(). Something, that was done by #94532 before.

serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think this needs a separate issue for discussion. Please open a new issue, and refer #78724 and #112358 (maybe even with citations).

We need to discuss what should be the end goal after the end of the deprecation period, and how deprecations canl help to transit the user code.

Comment thread Lib/test/test_struct.py Outdated
Comment thread Lib/test/test_struct.py Outdated
skirpichev marked this pull request as draft January 12, 2026 00:23
skirpichev changed the title gh-78724: deprecate incomplete initialization of struct.Struct() gh-143715: deprecate incomplete initialization of struct.Struct() Jan 12, 2026
skirpichev marked this pull request as ready for review January 12, 2026 06:09
This make format argument in the __init__() - optional.  If it's
missing, the object must be already initialized in __new__().
skirpichev requested a review from meadori January 13, 2026 01:35

Copy link
Copy Markdown
Member Author

CC @meadori per experts index.

Comment thread Modules/_struct.c Outdated
Comment thread Modules/_struct.c Outdated
Comment thread Modules/_struct.c Outdated

Copy link
Copy Markdown
Member Author

And what about the following case?

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

my_struct = MyStruct('<h')
my_struct.pack(12345)

There should emit a FutureWarning, because the current and the future code produce different results. MyStruct(5) should emit a DeprecationWarning, because it works in the current code, but will be error in future. ``MyStruct('>h')` should work without warnings.

@serhiy-storchaka, this seems too complex for me.

Look, we are going to that state: #94532. (I did a working patch to play with in skirpichev#17.) That means, eventually the Struct's __init__() will be no-op (and accept any arguments).

Thus, we should warn users on this pattern: explicit call of the __init__() on an object of some Struct subclass. Just in all above examples. Does make sense for you?

CC @vstinner

This catch current pattern for Struct's subclassing like

class MyStruct(Struct):
    def __init__(self):
        super().__init__('>h')
skirpichev marked this pull request as ready for review February 28, 2026 09:05
skirpichev dismissed vstinner’s stale review March 2, 2026 06:19

severe code changes

skirpichev removed their assignment Mar 2, 2026
Comment thread Lib/test/test_struct.py Outdated
Comment thread Modules/_struct.c Outdated
Comment thread Modules/_struct.c Outdated
Comment thread Lib/test/test_struct.py Outdated
skirpichev requested a review from vstinner March 2, 2026 13:52

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The overall change LGTM, but I have a few more minor comments.

Comment thread Lib/test/test_struct.py Outdated
Comment thread Modules/_struct.c Outdated
skirpichev and others added 2 commits March 2, 2026 17:23

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM.

@serhiy-storchaka: Do you want to review this change?

Copy link
Copy Markdown
Member

I tried to fix corner cases, but since all code and tests were rewritten, I created a separate PR #145580.

Copy link
Copy Markdown
Member Author

Ah, I forgot that format is positional or keyword argument. Will adapt code a bit.

skirpichev marked this pull request as draft March 7, 2026 06:42
skirpichev assigned skirpichev and unassigned skirpichev Mar 7, 2026

Copy link
Copy Markdown
Member Author

Apparently, this overflowed my bandwidth. I'm happy to see the issue is in more qualified hands now.

The #145580 looks ok for me, but I worry that such approach is much more complex just to satisfy the constraint:

  • It should be possible to write a user code compatible with future Python versions and with old Python versions which works without warnings during transitional period.

Transition could be handled with, say:

if sys.version_info < (3, 15):
    # old idiom:
    class MyStruct(struct.Struct):
        def __init__(self):
            super().__init__('>h')
else:
    # new idiom:
    class MyStruct2(struct.Struct):
        def __new__(cls):
            self = super().__new__(cls, '>h')
            return self

Thanks for reviews!

skirpichev closed this Mar 9, 2026
skirpichev deleted the deprecate-struct-init-2/78724 branch March 9, 2026 03:26
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL