Support pickling of some objects

[GodTamIt]

Fixes #96, supporting serialization via pythonize.

The (personal) use-case here is to be able to send types across process boundaries (i.e. large batches of Documents across to a process pool for some kind of processing).

[cjrh]

Hi @GodTamIt

Thank you for this! A few general comments:

The maintainers here have very little bandwidth, so larger PRs can take a lot of time to get reviewed. Don't get discouraged if it takes a while, this is normal. Generally speaking, smaller PRs travel exponentially faster than large PRs, in many repos not only this one.

The PR includes extra unnecessary changes like renaming Value to TvValue. I prefer large renames and similar refactors like this to be in separate PRs. (and if this particular rename was in its own PR, I would reject the PR as unnecessary).

I feel like the motivation for supporting pickle feels not yet strong enough to support a change of this size, and the taking on of new dependencies (in my opinion). A use-case would help to understand the motivation better. Is it to be able to move Fruit between processes/machines? Or a distributed system or similar? If we had our documentation already in place I'd ask you to add docs about the pickle feature which would help to explain more, but the docs aren't quite ready yet (#94).

This PR adds new dependencies. This will add to the time for review because I will go look at each of the dependencies to look for reasons to reject them. I have a high bar for taking on new dependencies. They are easy to add and very hard to remove. For instance, flexbuffers is a heavy C++ dependency we take on, just to support pickle. I would like us to try harder to make bincode work, or pythonize as suggested in #96? Or perhaps serde_pickle, etc.

I see you added the time crate, but seeing as we already have chrono, did you look at chrono::serde?

[GodTamIt]

Hi @GodTamIt

Thank you for this! A few general comments:

The maintainers here have very little bandwidth, so larger PRs can take a lot of time to get reviewed. Don't get discouraged if it takes a while, this is normal. Generally speaking, smaller PRs travel exponentially faster than large PRs, in many repos not only this one.

The PR includes extra unnecessary changes like renaming Value to TvValue. I prefer large renames and similar refactors like this to be in separate PRs. (and if this particular rename was in its own PR, I would reject the PR as unnecessary).

Ack. I appreciate the feedback here, and I've removed these changes. In the future, I will try to limit the change to only the necessary changes.

I feel like the motivation for supporting pickle feels not yet strong enough to support a change of this size, and the taking on of new dependencies (in my opinion). A use-case would help to understand the motivation better. Is it to be able to move Fruit between processes/machines? Or a distributed system or similar? If we had our documentation already in place I'd ask you to add docs about the pickle feature which would help to explain more, but the docs aren't quite ready yet (#94).

The primary motivation here is to be able to send these types (Document, SearchResult, etc.) across a process boundary. For example, concurrent.futures.ProcessPoolExecutor to operate on huge batches of Documents.

This PR adds new dependencies. This will add to the time for review because I will go look at each of the dependencies to look for reasons to reject them. I have a high bar for taking on new dependencies. They are easy to add and very hard to remove. For instance, flexbuffers is a heavy C++ dependency we take on, just to support pickle. I would like us to try harder to make bincode work, or pythonize as suggested in #96? Or perhaps serde_pickle, etc.

Ah, I didn't see the pythonizesuggestion until after posting this PR. I've elected for that, as it seems to be far lighter as a dependency and something that works.

I see you added the time crate, but seeing as we already have chrono, did you look at chrono::serde?

tantivy opts for the time crate internally instead. However, since tantivy likes to store it as a nanosecond timestamp, I've changed the code to just use an i64.

[GodTamIt]

This change was stacked on top of #99 (for equality support) and will need #98 to land before it can go in.

[GodTamIt]

Thanks for the reviews! This can land whenever