gh-143715: Deprecate incomplete initialization of struct.Struct()#145580
gh-143715: Deprecate incomplete initialization of struct.Struct()#145580serhiy-storchaka wants to merge 31 commits intopython:mainfrom
Conversation
* ``Struct.__new__()`` will require a mandatory argument (format) * Calls of ``__init__()`` method on initialized Struct are deprecated
This make format argument in the __init__() - optional. If it's missing, the object must be already initialized in __new__().
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
This catch current pattern for Struct's subclassing like
class MyStruct(Struct):
def __init__(self):
super().__init__('>h')
| :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`.) |
There was a problem hiding this comment.
| (Contributed by Sergey B Kirpichev in :gh:`143715`.) | |
| (Contributed by Serhiy Storchaka in :gh:`143715`.) |
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`.) |
There was a problem hiding this comment.
| (Contributed by Sergey B Kirpichev in :gh:`143715`.) | |
| (Contributed by Serhiy Storchaka in :gh:`143715`.) |
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?') |
There was a problem hiding this comment.
| totalsize = support.calcobjsize('2n3P1?') | |
| totalsize = support.calcobjsize('2n3P?') |
There was a problem hiding this comment.
It should be '2n3P?0P', with padding.
| super().__init__('>h') | ||
|
|
||
| my_struct = MyStruct('>h') | ||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') |
There was a problem hiding this comment.
I think this will be more clear, per Victor's suggestion:
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | |
| self.assertEqual(my_struct.format, '>h') |
(and in all cases below too)
There was a problem hiding this comment.
What if self->s_format and and self->s_codes are de-synchronized? The original test used Struct.pack().
| def __init__(self, *args, **kwargs): | ||
| super().__init__('>h') | ||
|
|
||
| my_struct = MyStruct('>h') |
There was a problem hiding this comment.
Why this not raises a warning? I think we should warn in all cases, where Struct.__init__() was explicitly called. // #143659 (comment)
There was a problem hiding this comment.
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 initializationworks now and will work in future.
|
|
||
| my_struct = MyStruct('>h') | ||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||
| my_struct = MyStruct(format='>h') |
There was a problem hiding this comment.
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.
| # 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',)) |
There was a problem hiding this comment.
Again, why no warnings here?
BTW, I doubt this usage pattern come from reality ;-)
There was a problem hiding this comment.
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.
2 participants
Struct.__new__()will require a mandatory argument (format)__init__()method on initialized Struct are deprecatedThis is an evolution of #143659, but since virtually all code and test were rewritten I created a new PR.
📚 Documentation preview 📚: https://cpython-previews--145580.org.readthedocs.build/