JuliaCollections / DataStructures.jl

Julia implementation of Data structures
https://juliacollections.github.io/DataStructures.jl/latest/
MIT License
679 stars 242 forks source link

Allow other int types for type container size #893

Closed KronosTheLate closed 5 months ago

KronosTheLate commented 5 months ago

This PR aims to solve https://github.com/JuliaCollections/DataStructures.jl/issues/891, and check if this issue occurs for other types as well. For the types listed [in the documentation](), the results are seen in the drop-down below.

Deque (implemented with an unrolled linked list) errors on `Deque{Float64}(Int32(10))`. Fixed CircularBuffer Silent error on CircularBuffer{Float32}(Int32(10)). Fixed CircularDeque (based on a circular buffer) errors on CircularDeque{Float32}(Int32(10)). Fixed Stack errors on Stack{Float64}(Int32(5)). Got fixed, I think with Deque Queue errors on Queue{Float64}(Int32(5)). Got fixed, I think with Deque Priority Queue No length parameter upon initialization Fenwick Tree Works for FenwickTree{Float64}(Int32(5)) and FenwickTree{Float64}(Int16(5)) No length parameter upon initialization for the following types Accumulators and Counters (i.e. Multisets / Bags) Disjoint Sets Binary Heap Mutable Binary Heap Ordered Dicts and Sets RobinDict and OrderedRobinDict (implemented with Robin Hood Hashing) SwissDict (inspired from SwissTables) Dictionaries with Defaults Trie Linked List and Mutable Linked List Sorted Dict, Sorted Multi-Dict and Sorted Set DataStructures.IntSet SparseIntSet DiBitVector Red Black Tree AVL Tree Splay Tree
KronosTheLate commented 5 months ago

Regarding your reviews, I though you were mainly changing my ecplixit conversion via Int(arg). But I had to add the lines in the first place to make it work. Have you tested your suggestions?

oxinabox commented 5 months ago

Have you tested your suggestions?

I have not. If you say they proved nesc then I will go with that. As my opinon is not strong

oxinabox commented 5 months ago

Do you need this backported to 0.18?

timholy commented 5 months ago

Fun fact: sometimes it's useful to use the Foo(n::Integer, args...) = Foo(Int(n), args...) pattern even for constructors that perform automatic conversion. The reason? It confines specialization-diversity to a short "stub" function, and the "long function" (the one that does the actual work) gets compiled for a narrow range of types. I.e., less latency.

That said, here the constructors are short so I don't think it's a big deal either way.

KronosTheLate commented 5 months ago

Do you need this backported to 0.18?

If you are asking me, then no ^_^