From 05f5452d9ad14fa5d03b0d83f158fcf456fb4590 Mon Sep 17 00:00:00 2001 From: Jeff Bezanson Date: Thu, 17 Aug 2017 17:28:47 -0400 Subject: [PATCH] fix #17886, `filter[!]` on dicts should pass 1 argument to the function --- NEWS.md | 3 +++ base/associative.jl | 57 ++++++++++++++++++++++++++++++++++--------- base/deprecated.jl | 3 +++ base/dict.jl | 18 +++++++++----- base/inference.jl | 2 +- base/pkg/read.jl | 2 +- base/repl/LineEdit.jl | 2 +- base/weakkeydict.jl | 9 +------ test/dict.jl | 4 +-- 9 files changed, 70 insertions(+), 30 deletions(-) diff --git a/NEWS.md b/NEWS.md index 4f001ae743fec..ea7294579758d 100644 --- a/NEWS.md +++ b/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 --------------------------- diff --git a/base/associative.jl b/base/associative.jl index ba9c7e3ebc10b..44ecd6faed3d5 100644 --- a/base/associative.jl +++ b/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,7 +327,7 @@ 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" @@ -335,10 +335,14 @@ Dict{Int64,String} with 2 entries: """ 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) @@ -346,11 +350,29 @@ function filter!(f, d::Associative) 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,7 +381,7 @@ 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" ``` @@ -367,9 +389,22 @@ Dict{Int64,String} with 1 entry: 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 diff --git a/base/deprecated.jl b/base/deprecated.jl index aff647f193df4..79375cc913450 100644 --- a/base/deprecated.jl +++ b/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 diff --git a/base/dict.jl b/base/dict.jl index 4dad63bed4d51..10c326b1358a1 100644 --- a/base/dict.jl +++ b/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 diff --git a/base/inference.jl b/base/inference.jl index 3d37c611c6aa6..0bfc01993cc2c 100644 --- a/base/inference.jl +++ b/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 diff --git a/base/pkg/read.jl b/base/pkg/read.jl index cd1f4461d1259..23945d4330210 100644 --- a/base/pkg/read.jl +++ b/base/pkg/read.jl @@ -134,7 +134,7 @@ function installed_version(pkg::AbstractString, prepo::LibGit2.GitRepo, avail::D end isempty(head) && return typemin(VersionNumber) - vers = collect(keys(filter((ver,info)->info.sha1==head, avail))) + vers = collect(keys(filter(#=ver,info=#p->p[2].sha1==head, avail))) !isempty(vers) && return maximum(vers) cache = Cache.path(pkg) diff --git a/base/repl/LineEdit.jl b/base/repl/LineEdit.jl index 7a355e80da9f2..2ee272e43a6aa 100644 --- a/base/repl/LineEdit.jl +++ b/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) diff --git a/base/weakkeydict.jl b/base/weakkeydict.jl index 17712ebc2f9f6..1ff64f5b172b1 100644 --- a/base/weakkeydict.jl +++ b/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) diff --git a/test/dict.jl b/test/dict.jl index 2f0a43ce9afd6..3954b342ed05f 100644 --- a/test/dict.jl +++ b/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