Skip to content

Implement skipmissing argument to levels #391

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

Merged
merged 3 commits into from
Sep 25, 2022
Merged

Conversation

nalimilan
Copy link
Member

The argument is added by DataAPI 1.10 (JuliaData/DataAPI.jl#46).
When skipmissing=true, the method for CategoricalArray can be slightly more efficient than the fallback defined in DataAPI
as it avoids calling unique.

The argument is added by DataAPI 1.10 (JuliaData/DataAPI.jl#46).
When `skipmissing=true`, the method for `CategoricalArray` can be
slightly more efficient than the fallback defined in DataAPI
as it avoids calling `unique`.
@nalimilan nalimilan requested a review from bkamins April 19, 2022 20:22
Comment on lines +2274 to +2277
@test levels(x, skipmissing=true) == ["b", "c", "a"]
@test levels(x, skipmissing=true) isa Vector{String}
@test levels(x, skipmissing=false) == ["b", "c", "a"]
@test levels(x, skipmissing=false) isa Vector{Union{String, Missing}}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a way to make this inferrable, even by wrapping this in a function. Not sure why. It's not terrible since the return type is inferred as Union{Vector{Union{String, Missing}}, Vector{String}}. But still...

@bkamins
Copy link
Member

bkamins commented Apr 23, 2022

There are some issues with Julia nightly 😞.

@nalimilan nalimilan closed this Sep 25, 2022
@nalimilan nalimilan reopened this Sep 25, 2022
@nalimilan nalimilan merged commit 847a745 into master Sep 25, 2022
@nalimilan nalimilan deleted the nl/missinglevels branch September 25, 2022 18:59
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