-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add file opener brotli.open
for Python
#806
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
❌ Build brotli 1.0.0#1481 failed (commit 154f35a988 by @youknowone) |
4e3cee0
to
1befb25
Compare
❌ Build brotli 1.0.0#1482 failed (commit 7debde6096 by @youknowone) |
❌ Build brotli 1.0.0#1483 failed (commit b3d2289b67 by @youknowone) |
Looks interesting. |
I copied it from CPython stdlib. So actually I didn't see that file. This is the CPython file: https://github.com/python/cpython/blob/master/Lib/gzip.py#L6 |
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to determine that you authored the commits in this PR. Maybe you used a different email address in the git commits than was used to sign the CLA? If someone else authored these commits, then please add them to this pull request and have them confirm that they're okay with them being contributed to Google. If there are co-authors, make sure they're formatted properly. In order to pass this check, please resolve this problem and then comment ℹ️ Googlers: Go here for more info. |
I splited CPython contributed part as a seperated commit but it causes CLA warning. what should i do in this case? |
❌ Build brotli 1.0.0#1501 failed (commit f6b351df41 by @youknowone) |
By needs, I released this one as a seperated project at this time Because dependencies are using Python3 for now, it needs a few tasks to finish this PR.
|
if newline is not None: | ||
raise ValueError("Argument 'newline' not supported in binary mode") | ||
|
||
gz_mode = mode.replace("t", "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
br_mode
?
@youknowone This would come in very handy, so I wanted to kindly ask whether you're interested in bringing this PR to completion. Thanks! |
@brechtm Since nobody having project ownership is interested in, I feel like spending more effort for this patch is not reasonable enough. You can install a separated project using pip https://pypi.org/project/brotli-file/ |
Thanks, @youknowone. I tried brotli-file, but unfortunately decompression is quite a bit slower than command-line brotli. I was planning on using brotli for compressing my CSV files which I read into pandas, but it seems |
There is an unreplied and unresolved review finding from eustas@ (with project ownership) here: https://github.com/google/brotli/pull/806/files/3462440035eb969771455d8812e5e7ed95e66a5f#r1058955829 |
The goal of this PR is adding
brotli.open
likegzip.open
.The patch is not completed yet - but the basic function compress/decompress is done as proof of concept.
file_test.py
is testing it.Before revising more, I want to know if this project is interested in this patch or not.