Skip to content
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

More AbstractDict interface and reduce invalidations/ambiguities #8

Merged
merged 10 commits into from
Aug 28, 2022

Conversation

Tokazama
Copy link
Member

Biggest improvements to interface were support for merge and mergewith. This required conditionally overloading some methods not defined in Base, but using the @static macro should prevent any issues there.

We were previously allowing any key type while internally converting between String and Symbol. The table and DataFrames community have learned that avoiding ambiguities with interchangeable Symbol and String keys takes some more explicit annotations (don't have an issue bit this comment mentions it). So we explicitly enforce that change in the type here. I don't think this breaks anyone's code but I think it is technically a breaking change, so I'm doing a v0.2 release on this one.

This enforces the key to be `Symbol` or `String`, which are the only
types compatible with dot access anyway. It also allows us to combine
a lot of code by using `_tokey(::PropertyDicts{K}, key)` to clearly
convert `key` to the appropriate type.
@codecov-commenter
Copy link

codecov-commenter commented Aug 28, 2022

Codecov Report

Merging #8 (b0fe6e9) into master (9fb29c6) will decrease coverage by 0.84%.
The diff coverage is 99.13%.

@@             Coverage Diff             @@
##            master       #8      +/-   ##
===========================================
- Coverage   100.00%   99.15%   -0.85%     
===========================================
  Files            1        1              
  Lines           71      118      +47     
===========================================
+ Hits            71      117      +46     
- Misses           0        1       +1     
Impacted Files Coverage Δ
src/PropertyDicts.jl 99.15% <99.13%> (-0.85%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Tokazama Tokazama merged commit 4f28c32 into master Aug 28, 2022
@Tokazama Tokazama deleted the more_dict_interface branch August 28, 2022 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants