strands-project/strands_qsr_lib

[qsr_lib] Switch from pickle to json

Closed this issue · 4 comments

I personally never use pickle and don't like that you can just pickle anything and send it as it is highly ambiguous and not very reusable. If we want to have a C++ client for the lib for example, I expect to run into serious problems. Also: http://www.benfrederickson.com/dont-pickle-your-data/

I propose to switch to json as I already do here as well. I'm happy to give that ago if you don't want to @yianni
The only question is which milestone this would go into.

Of course I am open for a discussions.

Need to meditate on this. I will get back to you as it is not majorly urgent, is it?

From what I recall pickle is used for serialization during ROS service call and nowhere else. Have you seen it anywhere else? Anyway if we agree to do so (need to read and think about it before making an opinion), I would probably go for 0.3 or something as I doubt that this particular internal mechanism has been touched by anyone else other than me so far.

Or 0.2.x

It's not urgent, no. Just thought that this needs to be done at some point if we want to enable the use of the lib via C++ as well as afaik pickle only serialises python objects.

@cdondrup

I had a go on this. Unfortunately, it is not straightforward the slightest because json only supports serialisation of booleans, numerics and strings. This means that the whole qsrlib_io package will have to provide json encoders and decoders, a fairly time consuming task. And even then it is still not exposed to C, but merely might allow to use python code in C (for example see here), which might be better doing it in python in the first place. Proper exposure to C might require rewriting in C the qsrlib_io package, something that is further time consuming.

I have no objection against doing it, but given the above reasons and since that there has not been even a single request for this, I am not going to do it.

Unless you want to give it a go, more than welcome to, I will mark it as wontfix and close the issue.