-
Notifications
You must be signed in to change notification settings - Fork 124
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
Hack to fix pickling #197
Hack to fix pickling #197
Conversation
attrs = self.__dict__.copy() | ||
del attrs["handle"] | ||
entries = self.intersection(self.bounds, objects=True) | ||
entries = [(item.id, item.bbox, item.object) for item in entries] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When interleaved=True
, this seems to require item.bbox
, but when interleaved=False
, this seems to require item.bounds
. I'm not even sure why there are two names for the same thing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed that seems like some kind of bug due to refactorings. It should be tracked down all the way to figure out what's going on.
Can you provide a test for this pickle support? I don't really want to merge it until it's tested. Also, I'm curious what would happen if the index's storage is a file instead of memory. |
This PR already contains testing for pickle support. We already had some testing, but we didn't actually test that the unpickled object matches the original object. I don't actually want to merge this PR as is, as it's kind of hacky and I don't fully understand why this is necessary. I'm hoping that someone smarter than me can propose a better solution. |
It would be some work, but I think you have everything needed in the libspatialindex C API to make a custom storage type for Python. That could be wired into more common Python things like buffers and such. |
I know very little about C/Cython so it might be better for someone else to do that. The goal of this PR was to push forward the discussion in #87 by proposing something so hacky that someone would investigate alternatives to avoid my current solution 😆 |
I'm wondering if we should revive this PR. Something hacky and slow that works is better than something that doesn't work. Being able to pickle an index is really important for any multiprocessing code on macOS/Windows. |
https://github.com/Toblerity/zope.index.rtree implemented a custom storage type for Zope's object store. You could probably revive this to wire it up to pickles. |
I assume this also doesn't work for cPickle? |
This is a hack that allows
Index
objects to be pickled. It works by copying the entire index into memory, pickling it, then rebuilding the entire index during unpickling. I hope that there is a more efficient, less hacky way to do this, but until then this at least lets users pickle rtree objects.Fixes #87