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

WIP: deprecate iteration on bare Associative, introduce PairIterator. #25013

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
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
109 changes: 63 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,13 @@ 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("next(::Associative, ::Any) is deprecated.")
display(stacktrace()); println() # TODO remove after problems fixed
next(PairIterator(a), i)
end

"""
empty(a::Associative, [index_type=keytype(a)], [value_type=valtype(a)])
Expand All @@ -155,7 +159,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 +188,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 +227,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 +254,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 +269,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 +387,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 +403,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 +418,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 +454,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 +474,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 +494,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 +505,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 +616,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
Loading