fix #17886, filter[!] on dicts should pass 1 argument to the function

NEWS.md

@@ -312,6 +312,9 @@ Deprecated or removed
   * `Base.cpad` has been removed; use an appropriate combination of `rpad` and `lpad`
     instead ([#23187]).
 
+  * `filter` and `filter!` on dictionaries now pass a single `key=>value` pair to the
+    argument function, instead of two arguments ([#17886]).
+
 Command-line option changes
 ---------------------------
 

base/associative.jl

@@ -317,7 +317,7 @@ end
     filter!(f, d::Associative)
 
 Update `d`, removing elements for which `f` is `false`.
-The function `f` is passed two arguments (key and value).
+The function `f` is passed `key=>value` pairs.
 
 # Example
 ```jldoctest
@@ -327,30 +327,52 @@ Dict{Int64,String} with 3 entries:
   3 => "c"
   1 => "a"
 
-julia> filter!((x,y)->isodd(x), d)
+julia> filter!(p->isodd(p.first), d)
 Dict{Int64,String} with 2 entries:
   3 => "c"
   1 => "a"
 ```
 """
 function filter!(f, d::Associative)
     badkeys = Vector{keytype(d)}(0)
-    for (k,v) in d
-        # don't delete!(d, k) here, since associative types
-        # may not support mutation during iteration
-        f(k,v) || push!(badkeys, k)
+    try
+        for (k,v) in d
+            # don't delete!(d, k) here, since associative types
+            # may not support mutation during iteration
+            f(k => v) || push!(badkeys, k)
+        end
+    catch e
+        return filter!_dict_deprecation(e, f, d)
     end
     for k in badkeys
         delete!(d, k)
     end
     return d
 end
 
+function filter!_dict_deprecation(e, f, d::Associative)
+    if isa(e, MethodError) && e.f === f
+        depwarn("In `filter!(f, dict)`, `f` is now passed a single pair instead of two arguments.", :filter!)
+        badkeys = Vector{keytype(d)}(0)
+        for (k,v) in d
+            # don't delete!(d, k) here, since associative types
+            # may not support mutation during iteration
+            f(k, v) || push!(badkeys, k)
+        end
+        for k in badkeys
+            delete!(d, k)
+        end
+    else
+        rethrow(e)
+    end
+    return d
+end
+
 """
     filter(f, d::Associative)
 
 Return a copy of `d`, removing elements for which `f` is `false`.
-The function `f` is passed two arguments (key and value).
+The function `f` is passed `key=>value` pairs.
 
 # Examples
 ```jldoctest
@@ -359,17 +381,30 @@ Dict{Int64,String} with 2 entries:
   2 => "b"
   1 => "a"
 
-julia> filter((x,y)->isodd(x), d)
+julia> filter(p->isodd(p.first), d)
 Dict{Int64,String} with 1 entry:
   1 => "a"
 ```
 """
 function filter(f, d::Associative)
     # don't just do filter!(f, copy(d)): avoid making a whole copy of d
     df = similar(d)
-    for (k,v) in d
-        if f(k,v)
-            df[k] = v
+    try
+        for (k, v) in d
+            if f(k => v)
+                df[k] = v
+            end
+        end
+    catch e
+        if isa(e, MethodError) && e.f === f
+            depwarn("In `filter(f, dict)`, `f` is now passed a single pair instead of two arguments.", :filter)
+            for (k, v) in d
+                if f(k, v)
+                    df[k] = v
+                end
+            end
+        else
+            rethrow(e)
         end
     end
     return df

base/deprecated.jl

@@ -1687,6 +1687,9 @@ end
 # issue #5148, PR #23259
 # warning for `const` on locals should be changed to an error in julia-syntax.scm
 
+# issue #17886
+# deprecations for filter[!] with 2-arg functions are in associative.jl
+
 # END 0.7 deprecations
 
 # BEGIN 1.0 deprecations

base/dict.jl

@@ -721,17 +721,23 @@ length(t::Dict) = t.count
 next(v::KeyIterator{<:Dict}, i) = (v.dict.keys[i], skip_deleted(v.dict,i+1))
 next(v::ValueIterator{<:Dict}, i) = (v.dict.vals[i], skip_deleted(v.dict,i+1))
 
-# For these Associative types, it is safe to implement filter!
-# by deleting keys during iteration.
-function filter!(f, d::Union{ObjectIdDict,Dict})
-    for (k,v) in d
-        if !f(k,v)
-            delete!(d,k)
+function filter_in_one_pass!(f, d::Associative)
+    try
+        for (k, v) in d
+            if !f(k => v)
+                delete!(d, k)
+            end
         end
+    catch e
+        return filter!_dict_deprecation(e, f, d)
     end
     return d
 end
 
+# For these Associative types, it is safe to implement filter!
+# by deleting keys during iteration.
+filter!(f, d::Union{ObjectIdDict,Dict}) = filter_in_one_pass!(f, d)
+
 struct ImmutableDict{K,V} <: Associative{K,V}
     parent::ImmutableDict{K,V}
     key::K

base/inference.jl

@@ -5294,7 +5294,7 @@ function find_sa_vars(src::CodeInfo, nargs::Int)
             end
         end
     end
-    filter!((v, _) -> !haskey(av2, v), av)
+    filter!(p -> !haskey(av2, p.first), av)
     return av
 end
 

base/repl/LineEdit.jl

@@ -895,7 +895,7 @@ end
 # source is the keymap specified by the user (with normalized keys)
 function keymap_merge(target,source)
     ret = copy(target)
-    direct_keys = filter((k,v) -> isa(v, Union{Function, KeyAlias, Void}), source)
+    direct_keys = filter(p -> isa(p.second, Union{Function, KeyAlias, Void}), source)
     # first direct entries
     for key in keys(direct_keys)
         add_nested_key!(ret, key, source[key]; override = true)

base/weakkeydict.jl

@@ -137,11 +137,4 @@ function next(t::WeakKeyDict{K,V}, i)  where V where K
     return (kv, (i, gc_token))
 end
 
-function filter!(f, d::WeakKeyDict)
-    for (k, v) in d
-        if !f(k, v)
-            delete!(d, k)
-        end
-    end
-    return d
-end
+filter!(f, d::WeakKeyDict) = filter_in_one_pass!(f, d)

test/dict.jl

@@ -511,7 +511,7 @@ let d = ImmutableDict{String, String}(),
 end
 
 # filtering
-let d = Dict(zip(1:1000,1:1000)), f = (k,v) -> iseven(k)
+let d = Dict(zip(1:1000,1:1000)), f = p -> iseven(p.first)
     @test filter(f, d) == filter!(f, copy(d)) ==
           invoke(filter!, Tuple{Function,Associative}, f, copy(d)) ==
           Dict(zip(2:2:1000, 2:2:1000))
@@ -636,7 +636,7 @@ Dict(1 => rand(2,3), 'c' => "asdf") # just make sure this does not trigger a dep
     @test 4 ∉ values(wkd)
     @test length(wkd) == 2
     @test !isempty(wkd)
-    wkd = filter!( (k,v) -> k != B, wkd)
+    wkd = filter!( p -> p.first != B, wkd)
     @test B ∉ keys(wkd)
     @test 3 ∉ values(wkd)
     @test length(wkd) == 1