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

Allow custom types to get narrow Union type with map (like promote_typejoin for Missing/Nothing) #38241

Open
oxinabox opened this issue Oct 30, 2020 · 26 comments
Labels
domain:missing data Base.missing and related functionality

Comments

@oxinabox
Copy link
Contributor

Consider this code:

julia> bar(::Nothing) = missing
bar (generic function with 1 method)

julia> bar(x) = x;

julia> xs = [1, 2, nothing, 4]
4-element Vector{Union{Nothing, Int64}}:
 1
 2
  nothing
 4

julia> ys = map(bar, xs)
4-element Vector{Union{Missing, Int64}}:
 1
 2
  missing
 4

We can see we get a nice array of small unions in the output.

But if I make my own similar singleton type, then the output is a Array{Any}.

julia> struct Null end

julia> foo(::Nothing) = Null();

julia> foo(x) = x;

julia> xs = [1, 2, nothing, 4]
4-element Vector{Union{Nothing, Int64}}:
 1
 2
  nothing
 4

julia> ys = map(foo, xs)
4-element Vector{Any}:
 1
 2
  Null()
 4

Is there something extra i need to do, or is the compiler cheating for Missing and Nothing?

@KristofferC
Copy link
Sponsor Member

KristofferC commented Oct 30, 2020

Is there something extra i need to do, or is the compiler cheating for Missing and Nothing?

There's a lot of cheating with those two (but not really by the compiler):

julia/base/missing.jl

Lines 41 to 70 in a80f903

function nonmissingtype_checked(T::Type)
R = nonmissingtype(T)
R >: T && error("could not compute non-missing type")
return R
end
promote_rule(T::Type{Missing}, S::Type) = Union{S, Missing}
promote_rule(T::Type{Union{Nothing, Missing}}, S::Type) = Union{S, Nothing, Missing}
function promote_rule(T::Type{>:Union{Nothing, Missing}}, S::Type)
R = nonnothingtype(T)
R >: T && return Any
T = R
R = nonmissingtype(T)
R >: T && return Any
T = R
R = promote_type(T, S)
return Union{R, Nothing, Missing}
end
function promote_rule(T::Type{>:Missing}, S::Type)
R = nonmissingtype(T)
R >: T && return Any
T = R
R = promote_type(T, S)
return Union{R, Missing}
end
convert(::Type{T}, x::T) where {T>:Missing} = x
convert(::Type{T}, x::T) where {T>:Union{Missing, Nothing}} = x
convert(::Type{T}, x) where {T>:Missing} = convert(nonmissingtype_checked(T), x)
convert(::Type{T}, x) where {T>:Union{Missing, Nothing}} = convert(nonmissingtype_checked(nonnothingtype_checked(T)), x)

Here is another example:

julia> struct Foo
          x::Ref{Union{Nothing, Float64}}
       end

julia> Foo(1)
Foo(Base.RefValue{Union{Nothing, Float64}}(1.0))

julia> struct MyMissing end

julia> struct Bar
          x::Ref{Union{MyMissing, Float64}}
       end

julia> Bar(1)
ERROR: MethodError: Cannot `convert` an object of type 
  Int64 to an object of type 
  Union{MyMissing, Float64}
Closest candidates are:
  convert(::Type{T}, ::T) where T at essentials.jl:171

@oxinabox
Copy link
Contributor Author

Nice.
Just adding
Base.promote_rule(T::Type{Null}, S::Type) = Union{S, Null}
doesn't seem to make it work.
It would be good to workout exactly what is needed and document it.

@KristofferC
Copy link
Sponsor Member

https://github.com/KristofferC/LazilyInitializedFields.jl/pull/1/files#diff-0abed87289859febd0e9c73389d5bb7fae46aa675acf8f1a468de7a3b9dcf0a1R95-R124 got me most of the way there. I still think there was something that was not perfect but don't really recall it.

@nalimilan
Copy link
Member

Being extensible wasn't the priority when we implemented this, but maybe it could be made easier and documented. There's also promote_typejoin which isn't part of the public API.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Oct 30, 2020

This is based on the explicit promote_typejoin rule, and not inferred by the compiler. It's also not extensible, sorry.

@vtjnash vtjnash closed this as completed Oct 30, 2020
@oxinabox
Copy link
Contributor Author

oxinabox commented Oct 30, 2020

Doesn't this issue remain open to document how to use promote_typejoin for this?

Or to make it extensible?

@oxinabox
Copy link
Contributor Author

Doesn't this issue remain open to document how to use promote_typejoin for this?

Or to make it extensible?

bump

@vtjnash
Copy link
Sponsor Member

vtjnash commented Dec 22, 2020

There's no "cheating" or uncertainty: it's the explicit implementation, and can't be extended, so there's not much to document?

@oxinabox
Copy link
Contributor Author

can't be extended

but just because it can't be extended now doesn't mean that it is impossible for making it extendable in the future.
Which would resolve this issue.
Not saying it is easy, but we have done non-easy enhancements to the language before.
Feature requests that are are hard are not normally closed, they are normally left open and untouched.
Feature requests that make no-sense, or that are a bad idea are normally closed.
Maybe this makes no sense or is a bad idea, but i don't understand why it is.

@oscardssmith oscardssmith reopened this Dec 22, 2020
@nalimilan nalimilan changed the title Custom Missing-like structs don't infer with map (but Missing does) Custom Missing-like structs don't get a narrow Union type with map (but Missing does) Dec 22, 2020
@iamed2
Copy link
Contributor

iamed2 commented Jan 4, 2021

I agree with @oxinabox; is there some rewording of this issue that could be reopened?

@nickrobinson251
Copy link
Contributor

nickrobinson251 commented Apr 19, 2021

Bump :) Can this be reopened or else a brief explanation be added on why this can't/won't be supported?

@cstjean
Copy link
Contributor

cstjean commented Apr 19, 2021

FWIW, we're relying on this behaviour as well in our product. @etpinard used this:

julia> Base.promote_typejoin(::Type{Blag}, ::Type{T}) where {T} =
           isconcretetype(T) || T === Union{} ? Union{T, Blag} : Any
julia> Base.promote_typejoin(::Type{T}, ::Type{Blag}) where {T} =
           isconcretetype(T) || T === Union{} ? Union{T, Blag} : Any

which works for us as end-users, although it's susceptible to method ambiguities, so I wouldn't use it inside of a library. It's both 1.5 and 1.6-compatible.

@quinnj
Copy link
Member

quinnj commented Apr 19, 2021

We've needed the Unioning behavior as well in a few packages recently, with code like this and this

@nalimilan nalimilan reopened this Apr 20, 2021
@StefanKarpinski
Copy link
Sponsor Member

Wouldn't it be possible to allow singleton types to opt into this with a trait?

@oscardssmith
Copy link
Member

I think that would be a good half-solution, but the better (and harder) solution would be to make singletons play nicer without having to ask.

@oxinabox
Copy link
Contributor Author

oxinabox commented Aug 6, 2021

better (and harder) solution would be to make singletons play nicer without having to ask.

Why?
It's different from other type promotion interaction.
It seems like it should be opt-in?

Or is it just always preferred?

@iamed2
Copy link
Contributor

iamed2 commented Aug 6, 2021

If it's possible to use a trait, is there a reason not to use Base.issingletontype as a default check (i.e. opt-in all singletons)?

@ParadaCarleton
Copy link
Contributor

Mentioned on the #statistics channel in the Slack -- being able to create custom Missing-like structs could be very useful for distinguishing between different kinds of missing data, e.g. data that are missing completely at random vs data that have been censored.

@nalimilan
Copy link
Member

This could probably be solved by having an AbstractMissing type in Base, even without making promote_typejoin extensible for non AbstractMissing types. (Note that currently Base hardcodes Missing/missing in quite a few places so the code would have to be made more generic, notably by using ismissing now that it's as good for inference as === missing.)

@oxinabox
Copy link
Contributor Author

oxinabox commented Dec 5, 2021

That's not good, as you can only have one supertype.
And if you are already using that then can't have another.

Also this behavior is not linked on any way to the notion of missing.

@nalimilan nalimilan changed the title Custom Missing-like structs don't get a narrow Union type with map (but Missing does) Allow custom types to get narrow Union type with map (like promote_typejoin for Missing/Nothing`) Mar 1, 2022
@nalimilan nalimilan changed the title Allow custom types to get narrow Union type with map (like promote_typejoin for Missing/Nothing`) Allow custom types to get narrow Union type with map (like promote_typejoin for Missing/Nothing) Mar 1, 2022
@KristofferC
Copy link
Sponsor Member

Small update, but the special casing of Nothing and Missing basically comes from this line:

_promote_typesubtract(@nospecialize(a)) = typesplit(a, Union{Nothing, Missing})

So if you for example do:

struct MySingleton end
foo(::Nothing) = MySingleton();
foo(x) = x;
xs = [1, 2, nothing, 4]
ys = map(foo, xs)
@show typeof(ys)
@eval Base Base._promote_typesubtract(@nospecialize(a)) = typesplit(a, Union{Nothing, Missing, $MySingleton})
ys = map(foo, xs)
@show typeof(ys)

it will show

Vector{Any} 
Vector{Union{MySingleton, Int64}} 

@KristofferC
Copy link
Sponsor Member

Speaking with Jameson a bit, a possible solution to this would be to just always go to a Union in the case of two types, and then go to Any instead of going directly to Any.

@quinnj
Copy link
Member

quinnj commented Oct 19, 2022

Yeah, that sounds like a pretty good compromise to me.

@cstjean
Copy link
Contributor

cstjean commented Oct 19, 2022

👍 from me too, but it doesn't help with

Mentioned on the #statistics channel in the Slack -- being able to create custom Missing-like structs could be very useful for distinguishing between different kinds of missing data, e.g. data that are missing completely at random vs data that have been censored.

We're also doing something similar (Vector{Floa64, Missing, SomethingLikeMissing), and will need to keep hacking something up until this is resolved...

@nalimilan
Copy link
Member

Are you sure you need to mix Missing and SomethingLikeMissing in the same vector? In my TypedMissings.jl experiment, missing is converted/promoted to a default TypedMissing value which doesn't specify the reason for missingness, so that you only ever need Vector{Float64, TypedMissing}.

@cstjean
Copy link
Contributor

cstjean commented Oct 19, 2022

That's an interesting approach. Our SomethingLikeMissing is AtLeast(x), so I guess we could use AtLeast(-Inf) as a missing replacement. It's ugly, though, I think we'd keep the hack for the time being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:missing data Base.missing and related functionality
Projects
None yet
Development

No branches or pull requests