Uh oh!
There was an error while loading. Please reload this page.
gh-132983: Simplify _zstd_exec() - #133775
Conversation
picnixz
left a comment
There was a problem hiding this comment.
Overall, I think it's a nice improvement but usually, C classes are read-only (e.g., _sha2.SHA2Type does not allow setting class variables afterwards; the same holds for lzma.LZMACompressor so it would be better to do the same for the Zstd* types)
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
serhiy-storchaka
left a comment
There was a problem hiding this comment.
It would be better to make classes implemented in C immutable.
emmatyping
left a comment
There was a problem hiding this comment.
Thanks! These are great refactors.
picnixz
left a comment
There was a problem hiding this comment.
You can add Py_TPFLAGS_IMMUTABLETYPE in this PR or in another.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Thanks @AA-Turner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
(cherry picked from commit bbe9c31) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
GH-133786 is a backport of this pull request to the 3.14 branch. |
cc @Rogdham (can't request review).
Follows on from #133670. Again best reviewed commit-by-commit. We remove around 100 lines of C code overall.
The last commit (
6a3a135(#133775)) only serves to merge two functions and contains no behaviour change, but does increase the diff size.add_type_to_module()function with anADD_TYPEmacro, usingPyModule_AddTypeinstead ofPyModule_AddObjectRef._zstdmodule.