Is your feature request related to a problem? Please describe.
ZSTD is used for compression in our serde process. However we don't need extra compression as we move to Protobuf.
ZSTD is usually a source of problems when installing PySyft with different hacks to solve it.
Describe the solution you'd like
Remove ZSTD dependency.
This will require removing the tests and its use in serde.
Describe alternatives you've considered
Protobuf covers compression.
Additional context
Hi @mccorby , I would like to work on this.
So, I guess I would need to remove the ZSTD functionalities in the test folder right? Is that it? Or do I need to remove ZSTD from the compression.py file in PySyft/syft/serde/ too?
We'll probably still want compression with Protobuf, but we can probably use zlib instead of zstd (if that's any easier to install.)
Could we proceed with two steps? First remove ZSTD, then add another compression tool?
Sounds good to me!
@r0cketr1kky : Ideally, ZSTD should be removed from any place where it's used. This should include:
And double check that it's not present either in any notebook (I think it's not).
Special care has to be taken when removing it from the production code (serde) as we want to leave the door open to have some other compression tool.
Hey, @mccorby I would like to work on this, is this issue taken up by someone?
@r0cketr1kky are working on this?
Yes, I am working on it. Would be great to get some help though. You can
reach out to me on Slack @RakshitNaidu
On Sat, Mar 7, 2020 at 12:03 AM Bolarinwa Saheed Olayemi <
[email protected]> wrote:
@r0cketr1kky https://github.com/r0cketr1kky are working on this?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/OpenMined/PySyft/issues/3134?email_source=notifications&email_token=AJLQTT37AF6AJKHV7X342O3RGE6WPA5CNFSM4LAGXKRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOCLYGI#issuecomment-595901465,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AJLQTTYSGWUBJL4RAYH5QM3RGE6WPANCNFSM4LAGXKRA
.
@r0cketr1kky sorry, a PR was approved on this issue. I think you can still check it and review if you have some suggestions. Would you work on adding zlib? @karlhigley kind of indicated that it will be a good replacement.
Since you have started working on this, adding zlib should be easy. Let me know so I'll stop working on it.
Not sure about how to add zlib to this. You can continue with this, I shall
drop out due to some other work.
On Sat, 7 Mar 2020 at 3:55 PM, Bolarinwa Saheed Olayemi <
[email protected]> wrote:
@r0cketr1kky https://github.com/r0cketr1kky sorry, a PR was approved on
this issue. I think you can still check it and review if you have some
suggestions. Would you work on adding zlib? @karlhigley
https://github.com/karlhigley kind of indicated that it will be a good
replacement.
Since you have started working on this, adding zlib should be easy. Let me
know so I'll stop working on it.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/OpenMined/PySyft/issues/3134?email_source=notifications&email_token=AJLQTT5XTZY7NZ4BFS4SRGDRGIOJLA5CNFSM4LAGXKRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEODVMSI#issuecomment-596072009,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AJLQTT44RCYUJ2NWNK73EGDRGIOJLANCNFSM4LAGXKRA
.
Sooo...who is working on this issue? @refactormyself @r0cketr1kky?
PR #3150 should have closed this issue
Then we can close this issue.