🤖 New build scheduled with the buildbot fleet by@kumaraditya303 for commita74cdbb 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

Apologies for the silence; I'll get to this by the end of this weekend.

LGTM too.

To be on the safe side, I'd recommendnot backporting this to 3.10, though it seems fine to backport to 3.11 if@pablogsal agrees. The bugs being fixed seem to be of the self-inflicted "well don't do that then" variety, so while it's definitely nice to have them fixed, it seems unlikely that they'll be blocking any real users ofStruct. And there's just enough possibility of unintended side-effects that it doesn't seem worth changing this in an already-released version of Python.

LGTM. I'm wondering if moving initialization from__init__ to__new__ can cause some user-facing backwards compatibility, say, wrt subclassing or something.

Yes, it can. This is fine, on Python 3.10:

>>>importstruct>>>classBob(struct.Struct):...def__init__(self,format):...super().__init__(format)...>>>b=Bob("!f")>>>b.pack(2.3)b'@\x1333'

But it breaks on this branch:

>>>importstruct>>>classBob(struct.Struct):...def__init__(self,format):...super().__init__(format)...>>>b=Bob("!f")Traceback (mostrecentcalllast):File"<stdin>",line1,in<module>File"<stdin>",line3,in__init__TypeError:object.__init__()takesexactlyoneargument (theinstancetoinitialize)

I'm finding it hard to imaginewhy anyone would be subclassingStruct like this, so it doesn't seem like a real issue if the behaviour changes in 3.11 or 3.12 (and in particular this certainly doesn't seem worth a deprecation period), but it does seem like sufficient reason to be careful in 3.10.

To be on the safe side, I'd recommendnot backporting this to 3.10, though it seems fine to backport to 3.11 if@pablogsal agrees.

I think is ok to backport to 3.11 👍 Not sure if it makes sense to mention the subclassing incompatibility in the what's new, though.

@kumaraditya303 Apologies, Kumar; I'm having second thoughts here. While common sense says that no-one will be subclassingstruct.Struct this way, experience says that someone somewhere will be, and it would be at least a bit irresponsible to change the behaviour.

Can you think of any ways that we could fix these bugs without the backward compatibility breakage?

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Thanks for the reviews!

Can you think of any ways that we could fix these bugs without the backward compatibility breakage?

I have made it backwards compatible by adding__init__ and added a test for subclassing.

I have made the requested changes; please review again@mdickinson@ambv

Thanks for the review, I'll make the changes by next weekend.

cc@mdickinson I have made the changes, I removed the code duplication and it is now only for 3.12+.

🤖 New build scheduled with the buildbot fleet by@mdickinson for commitd7ce7d8 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

The buildbots look happy. There are still three checks pending as of writing, but enough checks have passed to satisfy me. I'll merge now, but continue to watch those remaining three bots.

So I just hit this while trying to upgrade to 3.12. While subclassing works, subclassesMUST now match the same signature for__init__:

fromstructimportStructclassFloatStruct(Struct):def__init__(self):super().__init__('f')f=FloatStruct()

gives

Traceback (most recent call last):  File"/Users/rmay/test.py", line7, in<module>    f= FloatStruct()^^^^^^^^^^^^^TypeError:Struct() missing required argument 'format' (pos 1)

My actual use case was passing additional arguments, but either way fails.

I'm not here to necessarily vouch for why our implementation uses that, but it was seriously confusing not understanding why__init__ arguments were being checked before getting to my own__init__ implementation.