andyferris / Dictionaries.jl

An alternative interface for dictionaries in Julia, for improved productivity and performance
Other
278 stars 28 forks source link

Working `deepcopy` for `Dictionary` and `UnorderedDictionary` #107

Closed theogf closed 1 year ago

theogf commented 1 year ago

When using deepcopy on Dictionary with non isbits indices, there will be a mismatch between the hashes (unchanged) and the new objects (different hash). This PR should fix this by overloading deepcopy_internal as suggested in the Base docs.

I will just add some docs with an MWE where master would fail.

codecov[bot] commented 1 year ago

Codecov Report

Merging #107 (5157401) into master (34c17d0) will increase coverage by 0.03%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #107      +/-   ##
==========================================
+ Coverage   78.75%   78.78%   +0.03%     
==========================================
  Files          20       20              
  Lines        2320     2324       +4     
==========================================
+ Hits         1827     1831       +4     
  Misses        493      493              
Impacted Files Coverage Δ
src/Indices.jl 88.13% <100.00%> (+0.07%) :arrow_up:
src/UnorderedIndices.jl 88.78% <100.00%> (+0.10%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

theogf commented 1 year ago

Would you like me to tackle serialize/deserialize also in this PR or make a new one?

andyferris commented 1 year ago

It's generally preferable to have smaller, orthogonal PRs. If this is ready to merge we can do that and create a new one?

But also happy if you continue to work on this branch if that is better for you.

theogf commented 1 year ago

It's generally preferable to have smaller, orthogonal PRs. If this is ready to merge we can do that and create a new one?

But also happy if you continue to work on this branch if that is better for you.

LGTM then :)

theogf commented 1 year ago

I wrapped the @testsets in a global @testset, otherwise the tests will just stop at the first failed @testset. I can revert it if you don't like it.

andyferris commented 1 year ago

Yeah that should be fine

andyferris commented 1 year ago

Sweet, thanks @theogf :)