map applies function to unused pool levels

[nalimilan]

map applies the function to all values in the pool, whether they are used or not. This is notably a problem if you replace missing values:

julia> x = PooledArray([missing, 1])
2-element PooledArray{Union{Missing, Int64},UInt8,1,Array{UInt8,1}}:
  missing
 1

julia> x[1] = 0
0

julia> map(Int, x)
ERROR: MethodError: no method matching Int64(::Missing)

This is a difficult problem to fix, as it would require going over all values to check which ones are not used. One possibility would be to wrap the call in try... catch, store a special value in case of error, and check at the end whether that value was used. If so, call the function again to trigger the error.

[quinnj]

A couple of thoughts:

• I didn't even know we tried this optimization; one solution is just dropping it in favor of generic indexing + map
• It's really only an issue when you have a Union eltype, replace all entries of one of the Union types, and then map; so not terribly likely, but this is probably the most common situation you'd run into this. And in that regard, it'd probably be good to support dropmissing! and maybe replace! here which would excise Missing from the eltype
• A slightly less expensive solution than the try...catch proposal would be to put a anychanged::Bool flag in the PooledArray type; on any delete! or setindex! or other destructive operation, we'd set the flag; if the flag is set, we would fallback to map via indexing, only using the optimization if anychanged === false.

[nalimilan]

Yes at least we could avoid adding the try... catch to concrete types. But why do you say the try... catch approach would be expensive? If no error happens (most common case) the overhead should be quite small. And even if there's an error, it should be faster than falling back to the generic map.

[quinnj]

But won't we run into weird clashes if they have a legit error thrown in their mapping function? Like, it should fail, but now it silently succeeds and they get an empty array or something?

[nalimilan]

Well the approach I propose would assume the function is pure. So if it fails that's not an issue as we won't use its result.

[msavael]

This tripped me up in a slightly different situation: I had taken a slice of a PooledArray, then I wanted to map its values through a dictionary that (due to the program's logic) would only have values for entries in the slice. It errors because it is trying to look up values that were in the original PooledArray but aren't in the slice.

Perhaps a function to "trim"/ remove unused refs from a PooledArray would be generally useful?

[bkamins]

Yes - this function would be needed in many other situations also. Also a function to reorder the pool would be useful. Both are supported in CategoricalArrays.jl and it would be best if we had them exposed in DataAPI.jl so that we can write generic code.