Repository navigation
3.14: PEP-784 compression except zstd - #13992
Conversation
This comment has been minimized.
This comment has been minimized.
27b8b34 to
06181bc
Compare
This comment has been minimized.
This comment has been minimized.
06181bc to
e728c53
Compare
This comment has been minimized.
This comment has been minimized.
562bf17 to
24dafa3
Compare
This comment has been minimized.
This comment has been minimized.
|
There's a (small) merge conflict now due to allowlist changes. One request: Please don't force push here as that makes reviewing harder. We squash on merge anyway. |
|
Conflict should be resolved now. Sorry for the force-pushes earlier, I thought it was ok while the PR had the draft status. |
This comment has been minimized.
This comment has been minimized.
srittau
left a comment
srittau
left a comment
There was a problem hiding this comment.
Thanks, a few remarks below.
| @@ -0,0 +1,25 @@ | |||
| from _typeshed import WriteableBuffer | |||
There was a problem hiding this comment.
| from _typeshed import WriteableBuffer | |
| from _typeshed import Incomplete, WriteableBuffer |
See below.
There was a problem hiding this comment.
I just copied that file from _compression.pyi without any changes, should I make the requested changes on _compression.pyi as well?
There was a problem hiding this comment.
That would be great, thanks!
There was a problem hiding this comment.
That should be done now!
Do you have any thoughts on the code duplication between the two? Like a way to remove the duplication, or if I should add a comment on both files telling that the other should probably be updated at the same time.
There was a problem hiding this comment.
Maybe it would make sense to add a comment to the top of _compression.pyi to copy all changes to compression.*. I don't think the reverse direction is as important (and often the changes won't apply to older Python versions anyway.) Maybe add a comment to DecompressReader.decomp_factory in case someone actually adds a proper return type at some point.
There was a problem hiding this comment.
I have added the two comments, hopefully the wording is ok
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
2 participants
First PR about PEP-784 in sdlib:
bz2/gzip/lzma/zlibincompression.*_compressionintocompression._common._streams, and re-export the latter from the formerstubtest_allowlistsfor 3.14 accordingly (the additions are copied that already exists in the non-compressionmodules)For the last point, I did not find a better way than copying the file, because only one of the two modules exist depending on Python version (so cannot import from the other)▶️ feel free to suggest a better idea! (or tell if I should add a comment about that somewhere)
Adding
compression.zstdwill be done in a distinct PR.