From f2cc861c19fe93cfe2177cd219a3b70c6d96384f Mon Sep 17 00:00:00 2001
From: Anant Thazhemadam <anant.thazhemadam@gmail.com>
Date: Mon, 18 Oct 2021 21:46:17 +0530
Subject: [PATCH 1/3] missing: add `length(::SkipMissing)`

Define `length(::SkipMissing)` with `O(N)` complexity.
---
 base/missing.jl |  2 ++
 test/missing.jl | 11 +++++++++++
 2 files changed, 13 insertions(+)

diff --git a/base/missing.jl b/base/missing.jl
index e1988064aadc1..275207ddee14c 100644
--- a/base/missing.jl
+++ b/base/missing.jl
@@ -246,6 +246,8 @@ IteratorSize(::Type{<:SkipMissing}) = SizeUnknown()
 IteratorEltype(::Type{SkipMissing{T}}) where {T} = IteratorEltype(T)
 eltype(::Type{SkipMissing{T}}) where {T} = nonmissingtype(eltype(T))
 
+length(s::SkipMissing) = count(!ismissing, s.x)
+
 function iterate(itr::SkipMissing, state...)
     y = iterate(itr.x, state...)
     y === nothing && return nothing
diff --git a/test/missing.jl b/test/missing.jl
index 13ed684f1fc05..694b9ecc0890a 100644
--- a/test/missing.jl
+++ b/test/missing.jl
@@ -441,6 +441,17 @@ end
     @test collect(x) == [1, 2, 4]
     @test collect(x) isa Vector{Int}
 
+    @testset "length" begin
+        allmiss = Vector{Union{Int,Missing}}(missing, 10)
+        @test (length∘skipmissing)(allmiss) == 0
+
+        somemiss = [1, missing, 2, 3, 4, missing, 5, 6, 7, 8, 9, 10]
+        @test (length∘skipmissing)(somemiss) == 10
+
+        nomiss = rand(1:10, 10)
+        @test (length∘skipmissing)(nomiss) == 10
+    end
+
     @testset "indexing" begin
         x = skipmissing([1, missing, 2, missing, missing])
         @test collect(eachindex(x)) == collect(keys(x)) == [1, 3]

From 2ea054df0e029bb27171fb9fdf701491f74cb8b6 Mon Sep 17 00:00:00 2001
From: Anant Thazhemadam <anant.thazhemadam@gmail.com>
Date: Tue, 12 Jul 2022 12:18:23 +0530
Subject: [PATCH 2/3] missing: restrict `length(::SkipMissing)` to only
 `AbstractArray`

`length(::SkipMissing)`, as defined, can actually change the length
of some objects (like stateful iterators [1]).

Thus, restrict the method to work only on `AbstractArray`.

[1] `length(::SkipMissing)` changing the length of a `Stateful` iterator.
```
julia> itr = Iterators.Stateful([missing, 1, missing])
Base.Iterators.Stateful{Vector{Union{Missing, Int64}}, Union{Nothing, Tuple{Union{Missing, Int64}, Int64}}}(Union{Missing, Int64}[missing, 1, missing], (missing, 2), 0)

julia> count(!ismissing, skipmissing(itr).x) # proposed definition of length
1

julia> count(!ismissing, skipmissing(itr).x)
0
```
---
 base/missing.jl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/base/missing.jl b/base/missing.jl
index 275207ddee14c..5a8563e2bb1d3 100644
--- a/base/missing.jl
+++ b/base/missing.jl
@@ -246,7 +246,7 @@ IteratorSize(::Type{<:SkipMissing}) = SizeUnknown()
 IteratorEltype(::Type{SkipMissing{T}}) where {T} = IteratorEltype(T)
 eltype(::Type{SkipMissing{T}}) where {T} = nonmissingtype(eltype(T))
 
-length(s::SkipMissing) = count(!ismissing, s.x)
+length(s::SkipMissing{T}) where T <: AbstractArray = count(!ismissing, s.x)
 
 function iterate(itr::SkipMissing, state...)
     y = iterate(itr.x, state...)

From 5db9504f5c32ac207b8cdb1ce725dc7e46c1920f Mon Sep 17 00:00:00 2001
From: Anant Thazhemadam <anant.thazhemadam@gmail.com>
Date: Tue, 12 Jul 2022 14:50:10 +0530
Subject: [PATCH 3/3] test for error when length(::SkipMissing) is used with
 Stateful iterator

Add test checking if a `MethodError` is thrown while trying to use
`length(::SkipMissing)` with a Stateful iterator.
---
 test/missing.jl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/test/missing.jl b/test/missing.jl
index 694b9ecc0890a..70bf3df154a04 100644
--- a/test/missing.jl
+++ b/test/missing.jl
@@ -450,6 +450,9 @@ end
 
         nomiss = rand(1:10, 10)
         @test (length∘skipmissing)(nomiss) == 10
+
+        itr = Iterators.Stateful([missing, 1, missing, 1])
+        @test_throws MethodError (length∘skipmissing)(itr)
     end
 
     @testset "indexing" begin