Modified persistentId to allow an object, for the case in python where the id is a tuple

irmen / pickle

Java and .NET implementation of Python's pickle serialization protocol

MIT License

80 stars 6 forks source link

Modified persistentId to allow an object, for the case in python where the id is a tuple #15

Closed shaltielshmid closed 11 months ago

shaltielshmid commented 1 year ago

I've encountered a case when unpickling a file in C# where the persistentId is actually a tuple of items, and then when the function is called it just gets a string "System.Object[]" instead of the contents of the object.

irmen commented 1 year ago

Can you provide such a pickle file? I don't think I've seen one like that before.

Also, are you perhaps able to change the Java version as well, to keep in sync?

shaltielshmid commented 1 year ago

Generally, the library PyTorch return a tuple for the persistentId when using torch.save.

For an example, you can find one on the official pickle documentation page here. Their example uses a tuple as the persistentId.

shaltielshmid commented 1 year ago

Also, I'm adding another feature to allow for persistentId when pickling, I should update the PR soon.

shaltielshmid commented 1 year ago

Updated PR summary:

Modified persistentLoad to accept an object instead of a string, to allow for complex type ids.
Added a persistentId virtual function in pickler to allow using persistentIds when pickling.
Added an object deconstructor type, to allow serializing custom classes (the opposite of IObjectConstructor in unpickling). Technically this can be implemented as a custom pickler (using IObjectPickler), but I thought it would be cleaner if one doesn't have to deal with the opcodes in downstream code.

shaltielshmid commented 1 year ago

Update:

Added unit tests
Added implementation in Java + added unit tests as well

irmen commented 1 year ago

Thanks for working on this, I'll have a look soon

shaltielshmid commented 11 months ago

Hi @irmen , haven't heard from you in a few days. Wanted to check in about this PR, is there anything I can do to help merge it in?

irmen commented 11 months ago

Sorry I haven't found some time to actually review this yet. Will attempt to do so before the end of the week.

irmen commented 11 months ago

Looking good! Thank you for the contribution.

shaltielshmid commented 11 months ago

@irmen Thanks for merging! I saw you updated the version number to 1.5.0 - when do you think you'll upload the new version to NuGet?

Thanks!

irmen commented 11 months ago

done

shaltielshmid commented 11 months ago

Thanks!