Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow pickling simple instances of caches. #40

Closed
wants to merge 1 commit into from

Conversation

ncalexan
Copy link

@ncalexan ncalexan commented Jun 8, 2015

This is not particularly robust, but it handles (and tests) the simplest
examples. In general, pickle does not handle lambda functions, so this
is about the best that can be done. It's worth noting that this uses
a non-default pickle protocol: the regular one fails due to slots
with no getstate implementation. This non-default protocol is
specified with dumps(cache, -1): the -1 means "latest".

For a more robust pickling solution, try the dill module.

This is not particularly robust, but it handles (and tests) the simplest
examples.  In general, pickle does not handle lambda functions, so this
is about the best that can be done.  It's worth noting that this uses
a non-default pickle protocol: the regular one fails due to __slots__
with no __getstate__ implementation.  This non-default protocol is
specified with dumps(cache, -1): the -1 means "latest".

For a more robust pickling solution, try the dill module.
@tkem
Copy link
Owner

tkem commented Jun 9, 2015

Thanks, I wasn't aware of that, since I never used pickle myself.
Now, before I look into this any further, the unit tests (even more thanks for providing those) seem to fail on Python 3; could you please have a look what's wrong there?

@tkem
Copy link
Owner

tkem commented Jun 9, 2015

Please correct me if I'm wrong, but AFAICS, for the Cache base class, reverting to v1.0.0 with a non-lambda one function may also handle this for the simple case.

@tkem
Copy link
Owner

tkem commented Jun 9, 2015

As for pickling the Cache base class, please also take a look at 7bc51c6.

tkem added a commit that referenced this pull request Jun 17, 2015
@tkem
Copy link
Owner

tkem commented Jun 17, 2015

@ncalexan: you still there?

tkem added a commit that referenced this pull request Jun 17, 2015
tkem added a commit that referenced this pull request Jun 18, 2015
@tkem tkem closed this in 385ace8 Jun 18, 2015
@ncalexan
Copy link
Author

@tkmem: thanks for pushing this over the line.

On Thu, Jun 18, 2015 at 10:27 AM, Thomas Kemmer notifications@github.com
wrote:

Closed #40 #40 via 385ace8
385ace8
.


Reply to this email directly or view it on GitHub
#40 (comment).

@tkem
Copy link
Owner

tkem commented Jun 18, 2015

You're welcome ;-) Hope you don't mind that I didn't use your initial implementation, but this matched some refactorings regarding getsizeof/missing and the excessive use of lambdas I wanted to do anyway... And so I learned a little about pickling along the way, too.

@ncalexan
Copy link
Author

On Thu, Jun 18, 2015 at 11:46 AM, Thomas Kemmer notifications@github.com
wrote:

You're welcome ;-) Hope you don't mind that I didn't use your initial
implementation, but this matched some refactorings regarding getsizeof/
missing and the excessive use of lambdas I wanted to do anyway... And so
I learned a little about pickling along the way, too.

Not at all. I ended up using pylru since it was easier to pickle, so I'm
just glad a patch landed for next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants