How to check if an array contains categorical values?

[juliohm]

I used to write v isa CategoricalArray in user code to change the behavior and noticed that in the latest release of the package a view of a CategoricalArray is no longer a CategoricalArray. Is it safe to do eltype(v) <: CategoricalValue to check the condition in code? Perhaps a user function could be added to the api with this purpose?

[bkamins]

Is it safe to do eltype(v) <: CategoricalValue to check the condition in code?

It is not safe unfortunately.

Your question is in general hard that we need to resolve somehow - maybe @nalimilan has some more recent thoughts what would be best to do, as we are asked about it often.

[nalimilan]

Most of the time eltype(v) <: CategoricalValue should work, but depending on what you do you will likely get failures with Array{CategoricalValue{...}} objects (e.g. if you call levels).

The most correct solution is to use v isa Union{CategoricalArray, SubArray{<:Any, <:Any, <:CategoricalArray}} (or v isa CategoricalArrays.CatArrOrSub but it's not exported so there's no guaranty it will stay). I'm not sure what we could do to improve this except exporting CatArrOrSub and documenting it -- but we would need to find a better name.

[juliohm]

What about a iscategorical(v) trait function that does the above and hides from the user the internal detail? That way you can update the trait function in future releases without breaking things.

[nalimilan]

Well it would be equivalent to exporting CatArrOrSub, but with the drawback that it cannot be used for dispatch.

[juliohm]

What about exporting both? Would it make sense? Sometimes user code, specially in data science scripts end users just need to call a clean function to branch on behavior. I see the type for dispatch useful for a different audience. People who master Julia already.

…

On Tue, Mar 2, 2021, 07:54 Milan Bouchet-Valat ***@***.***> wrote: Well it would be equivalent to exporting CatArrOrSub, but with the drawback that it cannot be used for dispatch. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#332 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZQW3OOSSG4BIHH5IIKNA3TBS7WFANCNFSM4YNXTIVA> .

[nalimilan]

Why would iscategorical(v) be simpler for users than v isa CatArrOrSub (especially if we find a better name)?

[juliohm]

Maybe it is a matter of taste. Maybe it is because most beginners aren't even familiar with the existence of the operator isa in Julia. Writing if iscategorical(v) when iscategorical is well-documented in the package docs seems more natural to me, but of course you have the final decision. Also if for some reason the condition changes internally because the internal type is gone or split into two or more types, the complexity is hidden from the user who doesn't need to know about types. You basically shift the work to the maintainers of CategoricalArrays.jl who will be responsible to provide a working iscategorical across multiple releases for which the CatArrOrSub may not even exist. Changing CatArrOrSub is breaking so user code that does use CatArrOrSub will more likely need to change than user code that uses iscategorical.

[nalimilan]

Why would changing CatArrOrSub be more breaking than changing iscategorical?

[juliohm]

Say people are relying on the type for complex multiple dispatch and the type suddenly changes into a Union type or a subtype of another Array type. The code in the user side may get affected, isn't that correct? iscategorical will always be a function no matter what, and the behavior will be documented and guaranteed. I'm just sharing what I feel is more stable. For example, I'm reporting this issue now because my code was broken when I assumed v isa CategoricalArray was a safe check.

…

On Tue, Mar 2, 2021, 10:18 Milan Bouchet-Valat ***@***.***> wrote: Why would changing CatArrOrSub be more breaking than changing iscategorical? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#332 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZQW3KAWRTKRMUBBWBS77LTBTQRLANCNFSM4YNXTIVA> .

[nalimilan]

Yes that could create dispatch issues, but iscategorical couldn't be used at all for dispatch, so I'm not sure it's a strong argument. iscategorical is indeed less binding for us, but it also provides fewer possibilities to users.

[ablaom]

This issue, in case anyone hasn't noticed, is essentially a duplicate of #234, which is also open.

[juliohm]

Closing as duplicate