Are you sure it is not working out-of-the-box ?

Do you guys have any tutorial on what exactly do pickle.dump and pickle.load do? I mean, in what order, call which function. I googled but found nothing satisfying. @Trinkle23897 @duburcqa

Hum strange, I found the online Python documentation about Pickle quite well written.

Hum strange, I found the online Python documentation about Pickle quite well written.

Oh yes, I didn't notice this section. After reading it, everything is crystal clear.

Okay, so far there are 2 solutions to resolve this issue. The main issue is: pickle.loads will not initialize the self._meta first. When calling setstate = getattr(inst, "__setstate__", None) in pickle.loads, it will recursively call buffer.__getattr__ since self._meta always fails to fetch.

The first is to write __getstate__ and __setstate__ because it will not call __getattr__ which will cause RecursionError;

The second is to add a if before self._meta[key]:

def __getattr__(self, key):
    if '_meta' not in self.__dict__:
        raise AttributeError
    try:
        return self._meta[key]
    except KeyError:
        return AttributeError

But this will cause another overhead, though it doesn't need to rewrite the set/getattr. What's your opinion @duburcqa @youkaichao

The first is to write getstate and setstate

I vote for this, the pythonic way to do it. Pickling buffer does not work out of box because the customized behavior of buffer.__getattr__.

I think this is actually a stupid behavior of python. Why the hell does it invoke __getattr__ during pickle and unpickle? Just provide a default __setstate__ and __getstate__ makes the day much easier 👎

I vote for this, the pythonic way to do it.

Why would it be more pythonic ?

I also think it is more logical to override the pickle interface, or at least part of it, since it more directly solve the problème your are facing.

By "pythonic", I mean, do what python wants us to do, since python leaves these interfaces to achieve pickle.

By "pythonic", I mean, do what python wants us to do

I don't agree. Those interfaces have been designed to allow implementing much more complex behavior than one liners with default behavior except for a single attribute. So it is not clear to me if it is more appropriate to adapt the code to make the default work, or define custom interface, making pickle behavior less predictable for the user without reading the code.

Modifying the behavior of standard methods is almost always at last resort for me.

Okay, I withdraw the word "pythonic". Just because pickling buffer does not work out of box(buffer.__getattr__ is customized) so we need to use implement __setstate__ and __getstate__.

Btw, take a look at #180 when you are available 😄 @duburcqa

Codecov Report

Merging #182 into dev will increase coverage by 0.09%.
The diff coverage is 94.87%.

@@            Coverage Diff             @@
##              dev     #182      +/-   ##
==========================================
+ Coverage   89.16%   89.25%   +0.09%     
==========================================
  Files          39       39              
  Lines        2391     2402      +11     
==========================================
+ Hits         2132     2144      +12     
+ Misses        259      258       -1     

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f3b817...b289baf. Read the comment docs.

Nice to see you again! @duburcqa We are discussing about releasing version 0.3.0 after merging #182 and #179 :) It seems like a celebration for Tianshou having 2k stars (soon)!

Maybe it would be safe to wait one week or so before releasing it, just in case there is still a few bugs around. But I think the features are there.