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

Remove useless in() methods #128

Closed
wants to merge 1 commit into from
Closed

Remove useless in() methods #128

wants to merge 1 commit into from

Conversation

nalimilan
Copy link
Member

These are not actually needed for standard collections, which rely on isequal or == and hash. They create ambiguities and can even be incorrect, e.g. for ObjectIdDict. They could even be less efficient than the default if we stored hashes and implemented optimized hash(::CatValue, ::UInt) methods (#61). For reference, they have been introduced in 9638d31 (#66).

Fixes #127.

These are not actually needed for standard collections, which rely on isequal() or == and hash().
They create ambiguities and can even be incorrect, e.g. for ObjectIdDict. They could even be less
efficient than the default if we stored hashes and implemented optimized hash(::CatValue, ::UInt)
methods. For reference, they have been introduced in 9638d31.
@nalimilan
Copy link
Member Author

Merged as 412eb3b, but for some reason GitHub hasn't realized it.

@nalimilan nalimilan closed this Mar 1, 2018
@nalimilan nalimilan deleted the nl/in branch March 1, 2018 10:38
ExpandingMan pushed a commit to ExpandingMan/CategoricalArrays.jl that referenced this pull request Mar 1, 2018
These are not actually needed for standard collections, which rely on isequal() or == and hash().
They create ambiguities and can even be incorrect, e.g. for ObjectIdDict. They could even be less
efficient than the default if we stored hashes and implemented optimized hash(::CatValue, ::UInt)
methods. For reference, they have been introduced in 9638d31.
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.

1 participant