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

Add conditional and passmissing #89

Merged
merged 7 commits into from
Jan 17, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion src/Missings.jl
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module Missings

export allowmissing, disallowmissing, ismissing, missing, missings,
Missing, MissingException, levels, coalesce
Missing, MissingException, levels, coalesce, passmissing
Copy link
Member

Choose a reason for hiding this comment

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

I personally prefer Missings.propagate if we're still bikeshedding the name here.

Copy link
Member

Choose a reason for hiding this comment

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

I like propagate too, the only gripe is that it's quite long. But well...

Copy link
Member Author

Choose a reason for hiding this comment

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

But do we want to export it or not? If yes then propagate is ok, but Missings.propagate is a bit long.

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately propagate is quite general to claim it for this feature...

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 know, but that is why I thought the idea was propagatemissing which tab-completes.


using Base: ismissing, missing, Missing, MissingException

Expand Down Expand Up @@ -165,4 +165,24 @@ function levels(x)
levs
end

struct PassMissing{F} <: Function end
Copy link
Member

Choose a reason for hiding this comment

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

I think we probably want:

struct PassMissing{F} <: Function
    f::F
end


(::PassMissing{F})(xs...;kw...) where {F} =
any(ismissing, xs) || any(ismissing, values(values(kw))) ? missing : F(xs...; kw...)

"""
passmissing(f)

Return a function that returns `missing` if any of its positional or keyword arguments
are `missing` and otherwise applies `f` to those arguments.

# Examples
```jldoctest
julia> passmissing(sqrt).([missing, 4])
Copy link
Member

Choose a reason for hiding this comment

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

It would be useful to start with a simpler example with a single scalar (first non-missing, and then 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.

added

2-element Array{Union{Missing, Float64},1}:
missing
2.0
"""
passmissing(f::Base.Callable) = PassMissing{f}()
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be typeof(f)?

BTW, have you done some benchmarking to check there are no allocations?

Copy link
Member Author

Choose a reason for hiding this comment

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

AFAIK if I wanted to use typeof{f} then I would have to store f in a struct which would be slower and would allocate more.

I will post benchmarks for the current implementation in the main thread so that they do not disappear.

Copy link
Member

Choose a reason for hiding this comment

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

I think in most practical uses, the struct would never allocate. It's the same idea w/ the new iteration protocol: every call to iterate potentially returns a Tuple{T...}, but most of the time those tuples and even intermediate allocated objects don't even get allocated.


end # module
4 changes: 4 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -355,4 +355,8 @@ using Test, Dates, InteractiveUtils, SparseArrays, Missings

# MissingException
@test sprint(showerror, MissingException("test")) == "MissingException: test"

# Lifting
@test isequal(passmissing(sqrt).([missing, 4]), [missing, 2.0])
@test isequal(passmissing(parse)(Int, "a", base=missing), missing)
end