Replace TemporaryFile() with io.BytesIO()

[Ankit-Dhankhar]

Using similar syntax as torch_tensor_serializer/desearializer. #2357

[midokura-silvia]

Very nice improvement. Before merging, can you confirm me that there are unit tests for these functions? If not, can you please add some.

[kamathhrishi]

@midokura-silvia I don't think additional unit tests are required as tests for checking serialization already exists and they are passing with the proposed changes. What changes in this pr is how serialization is done.

[midokura-silvia]

Ok, I just confirmed that there is a unit test. Could we improve the unit test by using @pytest.mark.parametrize and call it for let's say 4 different arrays? Some combination of float and int arrays, with non-symmetric dimensions? The current choice of numpy.ones((10, 10)) feels a bit limited.

[Ankit-Dhankhar]

Sure I will add test cases.

[Ankit-Dhankhar]

I have added test cases for random integer arrays of non-symmetric dimensions.
But for array with float value it is giving assert tensor_serialized[0] != serde.NO_COMPRESSION this false.

[Ankit-Dhankhar]

I have added test-cases with requires_grad=True but tensor generated with torch.randn([dim]) fails this. Is this expected?

I have added test cases for random integer arrays of non-symmetric dimensions.
But for array with float value it is giving assert tensor_serialized[0] != serde.NO_COMPRESSION this false.

[midokura-silvia]

@robert-wagner any update from your side?

[iamtrask]

@midokura-silvia - if it's been ~7 days or more after checking in with someone who reviewed - and you feel the PR should be approved - you may go ahead and merge.

[iamtrask]

(in this case it's been 3 months - more than enough time)

[gmuraru]

Can I pick this up? @iamtrask

The requested changes have now been made.

[karlhigley]

@Ankit-Dhankhar I cloned your fork, squashed the commits in this PR, and rebased it on current PySyft master. You can see the resulting diff over here on the pr/2376 branch.

Sorry it's taken so long! Okay to force-push the result to your branch so we can finally get this PR merged? 😄