Skip to content

Commit

Permalink
WIP: deprecate iteration on bare Associative.
Browse files Browse the repository at this point in the history
Users should use `pairs(dict)` or `values(dict)` (or `keys(dict)`) to
disambiguate iteration result.
  • Loading branch information
Andy Ferris committed Dec 11, 2017
1 parent bd5d5a0 commit 91bbfac
Show file tree
Hide file tree
Showing 21 changed files with 134 additions and 104 deletions.
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,10 @@ Deprecated or removed
* The `sum_kbn` and `cumsum_kbn` functions have been moved to the
[KahanSummation](https://github.com/JuliaMath/KahanSummation.jl) package ([#24869]).

* Iteration (`next`, `eltype`, `in`, `map`, etc) on `Associative`s has been deprecated.
Explicitly nominate `pairs(dict)`, `values(dict)` or `keys(dict)` as appropriate
([#25013]).

Command-line option changes
---------------------------

Expand Down
108 changes: 62 additions & 46 deletions base/associative.jl
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,6 @@ const secret_table_token = :__c782dbf1cf4d6a2e5e3865d7e95634f2e09b5902__

haskey(d::Associative, k) = in(k, keys(d))

function in(p::Pair, a::Associative, valcmp=(==))
v = get(a,p[1],secret_table_token)
if v !== secret_table_token
valcmp(v, p[2]) && return true
end
return false
end

function in(p, a::Associative)
error("""Associative collections only contain Pairs;
Either look for e.g. A=>B instead, or use the `keys` or `values`
function if you are looking for a key or value respectively.""")
end

function summary(t::Associative)
n = length(t)
return string(typeof(t), " with ", n, (n==1 ? " entry" : " entries"))
Expand All @@ -44,26 +30,38 @@ struct ValueIterator{T<:Associative}
dict::T
end

summary(iter::T) where {T<:Union{KeySet,ValueIterator}} =
struct PairIterator{T<:Associative}
dict::T
end

function in(p::Pair, a::PairIterator, valcmp=(==))
v = get(a.dict, p[1], secret_table_token)
if v !== secret_table_token
valcmp(v, p[2]) && return true
end
return false
end

summary(iter::T) where {T<:Union{KeySet,ValueIterator,PairIterator}} =
string(T.name, " for a ", summary(iter.dict))

show(io::IO, iter::Union{KeySet,ValueIterator}) = show(io, collect(iter))
show(io::IO, iter::Union{KeySet,ValueIterator,PairIterator}) = show(io, collect(iter))

length(v::Union{KeySet,ValueIterator}) = length(v.dict)
isempty(v::Union{KeySet,ValueIterator}) = isempty(v.dict)
_tt2(::Type{Pair{A,B}}) where {A,B} = B
eltype(::Type{ValueIterator{D}}) where {D} = _tt2(eltype(D))
length(v::Union{KeySet,ValueIterator,PairIterator}) = length(v.dict)
isempty(v::Union{KeySet,ValueIterator,PairIterator}) = isempty(v.dict)
eltype(::Type{ValueIterator{D}}) where {D} = valtype(D)
eltype(::Type{PairIterator{D}}) where {D} = pairtype(D)

start(v::Union{KeySet,ValueIterator}) = start(v.dict)
done(v::Union{KeySet,ValueIterator}, state) = done(v.dict, state)
start(v::Union{KeySet,ValueIterator,PairIterator}) = start(v.dict)
done(v::Union{KeySet,ValueIterator,PairIterator}, state) = done(v.dict, state)

function next(v::KeySet, state)
n = next(v.dict, state)
n = next(PairIterator(v.dict), state)
n[1][1], n[2]
end

function next(v::ValueIterator, state)
n = next(v.dict, state)
n = next(PairIterator(v.dict), state)
n[1][2], n[2]
end

Expand Down Expand Up @@ -136,7 +134,12 @@ This includes arrays, where the keys are the array indices.
"""
pairs(collection) = Generator(=>, keys(collection), values(collection))

pairs(a::Associative) = a
pairs(a::Associative) = PairIterator(a)

function next(a::Associative, i)
println("Oops. ")
next(PairIterator(a), i)
end

"""
empty(a::Associative, [index_type=keytype(a)], [value_type=valtype(a)])
Expand All @@ -155,7 +158,7 @@ empty(a::Associative, ::Type{V}) where {V} = empty(a, keytype(a), V) # Note: thi

function copy(a::Associative)
b = empty(a)
for (k,v) in a
for (k,v) in pairs(a)
b[k] = v
end
return b
Expand Down Expand Up @@ -184,7 +187,7 @@ Dict{Int64,Int64} with 3 entries:
"""
function merge!(d::Associative, others::Associative...)
for other in others
for (k,v) in other
for (k,v) in pairs(other)
d[k] = v
end
end
Expand Down Expand Up @@ -223,7 +226,7 @@ Dict{Int64,Int64} with 3 entries:
"""
function merge!(combine::Function, d::Associative, others::Associative...)
for other in others
for (k,v) in other
for (k,v) in pairs(other)
d[k] = haskey(d, k) ? combine(d[k], v) : v
end
end
Expand All @@ -250,9 +253,9 @@ julia> keytype(Dict(Int32(1) => "foo"))
Int32
```
"""
keytype(::Type{Associative{K,V}}) where {K,V} = K
keytype(::Type{Associative{K, V}}) where {K, V} = K
keytype(a::Associative) = keytype(typeof(a))
keytype(::Type{A}) where {A<:Associative} = keytype(supertype(A))
keytype(::Type{A}) where {A <: Associative} = keytype(supertype(A))

"""
valtype(type)
Expand All @@ -265,10 +268,25 @@ julia> valtype(Dict(Int32(1) => "foo"))
String
```
"""
valtype(::Type{Associative{K,V}}) where {K,V} = V
valtype(::Type{A}) where {A<:Associative} = valtype(supertype(A))
valtype(::Type{Associative{K, V}}) where {K, V} = V
valtype(::Type{A}) where {A <: Associative} = valtype(supertype(A))
valtype(a::Associative) = valtype(typeof(a))

"""
pairtype(type)
Get the `Pair` type of an associative collection type. Behaves similarly to [`eltype`](@ref).
# Examples
```jldoctest
julia> pairtype(Dict(Int32(1) => "foo"))
Pair{Int32,String}
```
"""
pairtype(::Type{Associative{K, V}}) where {K, V} = Pair{K, V}
pairtype(::Type{A}) where {A <: Associative} = pairtype(supertype(A))
pairtype(a::Associative) = pairtype(typeof(a))

"""
merge(d::Associative, others::Associative...)
Expand Down Expand Up @@ -368,7 +386,7 @@ Dict{Int64,String} with 2 entries:
function filter!(f, d::Associative)
badkeys = Vector{keytype(d)}()
try
for pair in d
for pair in pairs(d)
# don't delete!(d, k) here, since associative types
# may not support mutation during iteration
f(pair) || push!(badkeys, pair.first)
Expand All @@ -384,7 +402,7 @@ end

function filter_in_one_pass!(f, d::Associative)
try
for pair in d
for pair in pairs(d)
if !f(pair)
delete!(d, pair.first)
end
Expand All @@ -399,7 +417,7 @@ 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)}()
for (k,v) in d
for (k,v) in pairs(d)
# don't delete!(d, k) here, since associative types
# may not support mutation during iteration
f(k, v) || push!(badkeys, k)
Expand Down Expand Up @@ -435,15 +453,15 @@ function filter(f, d::Associative)
# don't just do filter!(f, copy(d)): avoid making a whole copy of d
df = empty(d)
try
for pair in d
for pair in pairs(d)
if f(pair)
df[pair.first] = pair.second
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
for (k, v) in pairs(d)
if f(k, v)
df[k] = v
end
Expand All @@ -455,16 +473,14 @@ function filter(f, d::Associative)
return df
end

eltype(::Type{Associative{K,V}}) where {K,V} = Pair{K,V}

function isequal(l::Associative, r::Associative)
l === r && return true
if isa(l,ObjectIdDict) != isa(r,ObjectIdDict)
return false
end
if length(l) != length(r) return false end
for pair in l
if !in(pair, r, isequal)
for pair in pairs(l)
if !in(pair, pairs(r), isequal)
return false
end
end
Expand All @@ -477,8 +493,8 @@ function ==(l::Associative, r::Associative)
return false
end
if length(l) != length(r) return false end
for pair in l
if !in(pair, r, ==)
for pair in pairs(l)
if !in(pair, pairs(r), ==)
return false
end
end
Expand All @@ -488,7 +504,7 @@ end
const hasha_seed = UInt === UInt64 ? 0x6d35bb51952d5539 : 0x952d5539
function hash(a::Associative, h::UInt)
hv = hasha_seed
for (k,v) in a
for (k,v) in pairs(a)
hv ⊻= hash(k, hash(v))
end
hash(hv, h)
Expand Down Expand Up @@ -599,11 +615,11 @@ _oidd_nextind(a, i) = reinterpret(Int,ccall(:jl_eqtable_nextind, Csize_t, (Any,

start(t::ObjectIdDict) = _oidd_nextind(t.ht, 0)
done(t::ObjectIdDict, i) = (i == -1)
next(t::ObjectIdDict, i) = (Pair{Any,Any}(t.ht[i+1],t.ht[i+2]), _oidd_nextind(t.ht, i+2))
next(t::PairIterator{<:ObjectIdDict}, i) = (Pair{Any,Any}(t.dict.ht[i+1],t.dict.ht[i+2]), _oidd_nextind(t.dict.ht, i+2))

function length(d::ObjectIdDict)
n = 0
for pair in d
for pair in pairs(d)
n+=1
end
n
Expand Down
2 changes: 1 addition & 1 deletion base/deepcopy.jl
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ function deepcopy_internal(x::Dict, stackdict::ObjectIdDict)

dest = empty(x)
stackdict[x] = dest
for (k, v) in x
for (k, v) in pairs(x)
dest[deepcopy_internal(k, stackdict)] = deepcopy_internal(v, stackdict)
end
dest
Expand Down
5 changes: 5 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2186,6 +2186,11 @@ end
@deprecate_moved sum_kbn "KahanSummation"
@deprecate_moved cumsum_kbn "KahanSummation"

# PR #25013
@deprecate eltype(A::Type{Associative{K,V}}) where {K,V} pairtype(A)
@deprecate next(a::Associative, i) next(pairs(a), i)
#@deprecate Base.in(p::Pair, a::Associative, valcmp = (==)) Base.in(p, PairIterator(a), valcmp)

# END 0.7 deprecations

# BEGIN 1.0 deprecations
Expand Down
21 changes: 12 additions & 9 deletions base/dict.jl
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function show(io::IO, t::Associative{K,V}) where V where K
if !show_circular(io, t)
first = true
n = 0
for pair in t
for pair in pairs(t)
first || print(io, ',')
first = false
show(recur_io, pair)
Expand Down Expand Up @@ -139,9 +139,12 @@ Dict(ps::Pair{K}...) where {K} = Dict{K,Any}(ps)
Dict(ps::(Pair{K,V} where K)...) where {V} = Dict{Any,V}(ps)
Dict(ps::Pair...) = Dict{Any,Any}(ps)

pair_or_eltype(x) = eltype(x)
pair_or_eltype(x::Associative) = pairtype(x)

function Dict(kv)
try
associative_with_eltype((K, V) -> Dict{K, V}, kv, eltype(kv))
associative_with_eltype((K, V) -> Dict{K, V}, kv, pair_or_eltype(kv))
catch e
if !applicable(start, kv) || !all(x->isa(x,Union{Tuple,Pair}),kv)
throw(ArgumentError("Dict(kv): kv needs to be an iterator of tuples or pairs"))
Expand Down Expand Up @@ -193,7 +196,7 @@ empty(a::Associative, ::Type{K}, ::Type{V}) where {K, V} = Dict{K, V}()
# conversion between Dict types
function convert(::Type{Dict{K,V}},d::Associative) where V where K
h = Dict{K,V}()
for (k,v) in d
for (k,v) in pairs(d)
ck = convert(K,k)
if !haskey(h,ck)
h[ck] = convert(V,v)
Expand Down Expand Up @@ -711,8 +714,8 @@ function start(t::Dict)
return i
end
done(t::Dict, i) = i > length(t.vals)
@propagate_inbounds function next(t::Dict{K,V}, i) where {K,V}
return (Pair{K,V}(t.keys[i],t.vals[i]), skip_deleted(t,i+1))
@propagate_inbounds function next(t::PairIterator{Dict{K,V}}, i) where {K,V}
return (Pair{K,V}(t.dict.keys[i],t.dict.vals[i]), skip_deleted(t.dict,i+1))
end

isempty(t::Dict) = (t.count == 0)
Expand Down Expand Up @@ -756,11 +759,11 @@ ImmutableDict
ImmutableDict(KV::Pair{K,V}) where {K,V} = ImmutableDict{K,V}(KV[1], KV[2])
ImmutableDict(t::ImmutableDict{K,V}, KV::Pair) where {K,V} = ImmutableDict{K,V}(t, KV[1], KV[2])

function in(key_value::Pair, dict::ImmutableDict, valcmp=(==))
function in(key_value::Pair, dict::PairIterator{<:ImmutableDict}, valcmp=(==))
key, value = key_value
while isdefined(dict, :parent)
if dict.key == key
valcmp(value, dict.value) && return true
if dict.dict.key == key
valcmp(value, dict.dict.value) && return true
end
dict = dict.parent
end
Expand Down Expand Up @@ -792,7 +795,7 @@ end

# this actually defines reverse iteration (e.g. it should not be used for merge/copy/filter type operations)
start(t::ImmutableDict) = t
next(::ImmutableDict{K,V}, t) where {K,V} = (Pair{K,V}(t.key, t.value), t.parent)
next(::PairIterator{ImmutableDict{K,V}}, t) where {K,V} = (Pair{K,V}(t.key, t.value), t.parent)
done(::ImmutableDict, t) = !isdefined(t, :parent)
length(t::ImmutableDict) = count(x->true, t)
isempty(t::ImmutableDict) = done(t, start(t))
Expand Down
2 changes: 1 addition & 1 deletion base/docs/utils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ repl_corrections(s) = repl_corrections(STDOUT, s)
const symbols_latex = Dict{String,String}()
function symbol_latex(s::String)
if isempty(symbols_latex)
for (k,v) in Base.REPLCompletions.latex_symbols
for (k,v) in pairs(Base.REPLCompletions.latex_symbols)
symbols_latex[v] = k
end
end
Expand Down
8 changes: 4 additions & 4 deletions base/env.jl
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pop!(::EnvDict, k::AbstractString) = (v = ENV[k]; _unsetenv(k); v)
pop!(::EnvDict, k::AbstractString, def) = haskey(ENV,k) ? pop!(ENV,k) : def
delete!(::EnvDict, k::AbstractString) = (_unsetenv(k); ENV)
setindex!(::EnvDict, v, k::AbstractString) = _setenv(k,string(v))
push!(::EnvDict, k::AbstractString, v) = setindex!(ENV, v, k)
push!(::EnvDict, k::AbstractString, v) = setindex!(ENV, v, k) # Should this be a `Pair`?

if Sys.iswindows()
start(hash::EnvDict) = (pos = ccall(:GetEnvironmentStringsW,stdcall,Ptr{UInt16},()); (pos,pos))
Expand All @@ -92,7 +92,7 @@ if Sys.iswindows()
end
return false
end
function next(hash::EnvDict, block::Tuple{Ptr{UInt16},Ptr{UInt16}})
function next(hash::PairIterator{EnvDict}, block::Tuple{Ptr{UInt16},Ptr{UInt16}})
pos = block[1]
blk = block[2]
len = ccall(:wcslen, UInt, (Ptr{UInt16},), pos)
Expand All @@ -109,7 +109,7 @@ else # !windows
start(::EnvDict) = 0
done(::EnvDict, i) = (ccall(:jl_environ, Any, (Int32,), i) === nothing)

function next(::EnvDict, i)
function next(::PairIterator{EnvDict}, i)
env = ccall(:jl_environ, Any, (Int32,), i)
if env === nothing
throw(BoundsError())
Expand All @@ -126,7 +126,7 @@ end # os-test
#TODO: Make these more efficent
function length(::EnvDict)
i = 0
for (k,v) in ENV
for (k,v) in pairs(ENV)
i += 1
end
return i
Expand Down

0 comments on commit 91bbfac

Please sign in to comment.