Closed mingmingrr closed 2 years ago
Hi!
Thanks, this is quite a mouthful. Several thousand lines of code that I won't be able to review in great detail. And that I really don't feel like taking responsibility for in terms of bug fixing, etc.
I can see the benchmarks provided in the PR and the data structure does indeed have some appealing characteristics for certain use cases. What were the main motivations for you to create it? If this was merged would you be willing to take on the ownership to fix any issues in the sequence implementation?
An option to merging all this into pyrsistent could be to release it as its own library and have pyrsistent depend on it with a thin wrapper or similar. Did you consider that?
I'd be willing to maintain anything related to psequence. Haven't thought of releasing it as its own library but that seems like a decent option. I would have to copy class custom_build_ext
from setup.py
, and pyrsistent can import psequence
, PSequence
, and sq
. If that's your preference then I'll close this PR and move this into a new library, otherwise I'd be okay with being a maintainer.
As far as motivations, I wanted something similar to Haskell's Data.Seq
with faster merge and split, which, as far as I know, doesn't currently exist in Python.
Thanks, I think trying it out as a separate library that is imported is a first good step!
Just an update, this is now published on pypi: https://pypi.org/project/pyrsistent-extras
Cool! I've added a reference to your repo in the README.
Here's a sequence type implemented with FingerTrees.
I didn't like how PVector was structured so I moved everything into its own submodule. That way the C implementation can import the base class and "inherit" its docstrings. I could merge them into a single file like _pvector.py, or create a separate PR to copy these changes to PVector.
Other implementation differences from PVector:
view, splitat, chunksof
) to take advantage of fast splittingmset
can take 2-tuples instead of pairs of argsviewleft/viewright
instead ofpopleft/pop
, sincepop
has different behavior in evolversNot implemented:
CheckedPSequence
Some benchmarks:
Code: https://gist.github.com/mingmingrr/e9e9245b28672c824a8b492002d0780c Axes are logarithmic. X axis is container size, Y axis is time in seconds. Construction: ![construction](https://user-images.githubusercontent.com/12855406/188293951-2c314900-a4ab-4d4e-a608-ab18b251c5de.png) Get first element: ![get_left](https://user-images.githubusercontent.com/12855406/188293956-508e9992-80b9-46a9-876b-18d46f306040.png) Get middle element: ![get_middle](https://user-images.githubusercontent.com/12855406/188293957-4c1e66be-de8d-4fbe-bc13-8449693932c0.png) Get last element: ![get_right](https://user-images.githubusercontent.com/12855406/188293958-7bbfe786-f6f0-4c4b-9e10-c60be84695ab.png) Slice continguous chunk: ![slice_chunk](https://user-images.githubusercontent.com/12855406/188293963-f8856ee0-3939-4134-ab75-4eaa235c8a3d.png) Slice with step: ![slice_step](https://user-images.githubusercontent.com/12855406/188293964-5d82fe02-5083-48e5-ad31-beb1d06160c0.png) Insert first element: ![insert_left](https://user-images.githubusercontent.com/12855406/188293959-58df5b98-0f73-4a04-8976-fb495a131685.png) Insert middle element: ![insert_middle](https://user-images.githubusercontent.com/12855406/188293960-c0f1a2f2-9975-40a0-9181-f462f3b9a5af.png) Insert last element: ![insert_right](https://user-images.githubusercontent.com/12855406/188293961-3927a5f8-d43c-4548-b787-3630d892a6a4.png) Delete first element: ![delete_left](https://user-images.githubusercontent.com/12855406/188293952-7419939a-d202-4c85-bd58-a77a777819c4.png) Delete middle element: ![delete_middle](https://user-images.githubusercontent.com/12855406/188293953-14ff6e8e-d01a-4d61-8a4c-9220fb58e37d.png) Delete last element (couldn't find a `pvector.pop()`): ![delete_right](https://user-images.githubusercontent.com/12855406/188293954-78e87214-9636-4c9d-9a26-da76efa451d1.png) Extend: ![extend](https://user-images.githubusercontent.com/12855406/188293955-2d59f03e-b594-4908-91ce-0505de351b69.png) `__mul__` (psequence also takes `O(log(n)log(k))` memory): ![repeat](https://user-images.githubusercontent.com/12855406/188293962-313401a4-fee1-48b8-b96a-e133ba9b9bb2.png)