| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
sorry for delay, all the docstring/comments were copy pasted from the zlib function. I guess the old function had an old style that is no longer wanted. for the return type in the docstring, we can settle with "positive integer" https://docs.python.org/3/library/zlib.html#zlib.crc32 |
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks!
Sorry, something went wrong.
|
@AA-Turner , I merged all your suggestions. for the versionadded: next, I thought changes on main would ship in 3.14? I am too late for 3.14 release? |
Sorry, something went wrong.
|
'next' is updated by the Release Manager for each version, it's a convenience tool that we use internally. We can still get this in for 3.14, yes. A |
Sorry, something went wrong.
|
regarding performance, don't mind that for now, I will come back later. in principle
liblzma had code to start using hardware crc instructions soon ... and I see they just made a new stable release few days ago https://github.com/tukaani-project/xz/releases |
Sorry, something went wrong.
|
it appears the Ubuntu (free-threading) / build and test (ubuntu-24.04-arm is flaky |
Sorry, something went wrong.
|
I'll re-run it. |
Sorry, something went wrong.
|
build passed, ready to merge |
Sorry, something went wrong.
|
Do you want to include your man.com email in the commit? Currently GH lists it as a co-authored-by, but the commit itself will be under your GMail address. |
Sorry, something went wrong.
|
Please don't mind the email. |
Sorry, something went wrong.
|
ping, can this be merged? |
Sorry, something went wrong.
|
@merwok @AA-Turner are you able to review? I've rebased on main and redid the chance, since it's been 3 months and main had changed. |
Sorry, something went wrong.
|
@gpshead you've replied multiple times to my questions on improving compression code with "PR welcome". maybe you would be interested in reviewing this PR? |
Sorry, something went wrong.
|
This PR is stale because it has been open for 30 days with no activity. |
Sorry, something went wrong.
|
I’m a little confused by the issue linked from this PR. The issue is about using zlib-ng as the underlying library for our zlib module. This PR exposes a function from lzma. What is the link to zlib-ng or zlib module? |
Sorry, something went wrong.
Hello, (first part) It's pretty simple. There is no downside or issue that can come from exposing this simple function that's been stable for decades. (second part) zlib has been unmaintained for most of the decade, it's not using hardware instructions to calculate the CRC and it's never gonna be updated. It started being replaced by zlib-ng for massive performance improvements, see the linked issue, unfortunately there are plenty of systems/OS that will remain stuck on zlib for a long time. Once the lzma crc function is exposed, this will allow applications to use it instead of the slow zlib crc. |
Sorry, something went wrong.
|
We do not want to increase our reliance on liblzma due to its license and general trust in the project. It'd be far easier to add our own fast internal crc32 implementation if that is desired at this point. |
Sorry, something went wrong.
I'm sorry but this replies makes no sense whatsoever. How is there any problem with xz-utils license or trust? especially in the context of one CRC function? It's free software and it's the baseline of multiple Linux distributions to distribute Linux packages. Python making its own CRC implementation is not on the table, this would require to write advanced C code with intrinsic/assembly per platform, which is completely outside of the scope of python, where maintainers can't even agree on the signature of one function. It's absurd to claim that the code should be in Python. EDIT: if the concern is not wanting the python stdlib (binascii and co) to use lzma.crc because zlib.crc is more free, I'm happy to just expose the function, and not any make further PRs to the interpreter to use it. |
Sorry, something went wrong.
Sorry, I don't understand this. I have absolutely no stake in this bug, but:
Feel free to reject it on other rationale (as I said, I have no opinion on this) but I don't think these two sound right. If there's something that we need to do more on the xz side, please let me know as well. |
Sorry, something went wrong.
|
Sorry, a quick search was showing me GPL, looking at the repo its a big mix of things and the strictly liblzma part is not GPL. regardless, my larger point is that we should not depend on a compression library for crc32 - it isn't the primary purpose of the library. we should just ship our own performant functions. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Hello,
This is a simple PR to expose the crc32 function from the lzma library.
The code and tests are very similar to the crc in binascii and zlib. I don't think it's too controversial.
I came across this while looking at compression libraries for #91349
crc32 is always available from the lzma library (xz-utils/liblzma.so). I think it should be exposed.
there are more hashing functions available (namely crc64) but they need compilation/runtime checks to verify whether they are available.
Regards.
📚 Documentation preview 📚: https://cpython-previews--131721.org.readthedocs.build/