Define OrderStyle for Union{}

[bkamins]

Removes ambiguity:

julia> Base.OrderStyle(Union{})
ERROR: MethodError: Base.OrderStyle(::Type{Union{}}) is ambiguous. Candidates:

This error is relevant as with current unique! definition that relies on OrderStyle one can have a problem in corner cases. E.g.:

julia> [i for i in ["1"] if i isa Int]
0-element Array{Union{},1}

I propose to make this case Unordered() as probably this was the intention of the original implementation, but probably it does not matter much which option is chosen.

[bkamins]

Actually maybe Union{} should be Ordered
as it is natural to assume that if A <: B then if B is Ordered then A is Ordered,
and for Unordered such a relationship does not have to hold.

[bkamins]

@nalimilan - I cannot see your comment I got via e-mail, but I can answer it. Initially I thought to use Unordered, but I changed my mind and I recommend to use Ordered for Union{}.

[nalimilan]

I guess Ordered() is OK since issorted(Union{}[]) is true.

[bkamins]

@timholy merging this is required I think as otherwise after #36449 this missing definition will manifest itself in practice (we already have a case of this in DataFrames.jl).

[bkamins]

Also my question is - what is the exact meaning of this trait? Should it be related to < or isless? The question is that I assume it is isless, and in this case Union{Missing, Real}, Union{Missing, Symbol} and Union{Missing, AbstractString} also can be considered as Ordered() I think.

[timholy]

Thanks @bkamins!

I'd merge this as-is except that I see it's been marked for triage.

[KristofferC]

What's there to triage here @ararslan?

[timholy]

Also my question is - what is the exact meaning of this trait? Should it be related to < or isless? The question is that I assume it is isless, and in this case Union{Missing, Real}, Union{Missing, Symbol} and Union{Missing, AbstractString} also can be considered as Ordered() I think.

Good question. There's no unique answer, but I'd take "whatever sort does" as the reference. Since the default comparison function for sort is isless, I'd agree with you.

[nalimilan]

Good point. Maybe let's change this in another PR to avoid holding this one back?

[bkamins]

OK - I will open a separate PR for this when this is merged.

[timholy]

Bump @ararslan, does this actually need triage?

[timholy]

Thanks as always, @bkamins !

[bkamins]

@nalimilan, so before opening a PR, maybe let us discuss if we want to add Ordered trait to:

• other types defined in base that support fast isless, I do not want to list them here but Char is an example
• adding union with Missing everywhere for them (and then defining the method only for Missing to avoid dispatch ambiguity)

This will not be used much probably (so we could ignore the fact that we do not have this trait for them), but for consistency it would be cleaner to have it. I am not sure what is best (I am leaning towards leaving things as is and that is why I am commenting here and not opening a PR)

[nalimilan]

I agree that it seems the trait should be defined for all types that support isless, including Union{T, Missing}. The meaning of the trait should also be documented (notably the fact that it's used to choose fast paths).

[bkamins]

@ararslan - is this going to be backported into Julia 1.5.1 release?

[KristofferC]

This didn't have a backport label so it was not included in 1.5.1.

[bkamins]

I know, but the original discussion was to backport it it - and that is why I am asking.

My question is if then the plan is to have it backported to 1.5 at some point or it will be in 1.6.

[nalimilan]

Tentatively adding the backport label so that at least it's considered for 1.5.2.