Is push! implemented correctly?

[torfjelde]

I just recently discovered that push! is implemented for Dict and it states the following in the docstring:

Because of this I sort of (maybe naively) expected push!(::OrderedDict, ::Pair) to always insert entries at the end, but this doesn't seem to be the case:

julia> using OrderedCollections

julia> d = OrderedDict(:x => 1)
OrderedDict{Symbol, Int64} with 1 entry:
  :x => 1

julia> push!(d, :y => 2)
OrderedDict{Symbol, Int64} with 2 entries:
  :x => 1
  :y => 2

julia> push!(d, :x => 3)  # Expected `:x => 3` to be after `:y => 2` here
OrderedDict{Symbol, Int64} with 2 entries:
  :x => 3
  :y => 2

Is this intentional?

[timholy]

Presumably this is only an issue with pre-existing keys. To me, it's not entirely obvious what the right answer is. Anyone else have any thoughts?

For reference, Python now guarantees that their dicts are ordered and here's what they do:

>>> d = {'x':1, 'y':2}
>>> d.
d.clear(       d.fromkeys(    d.items(       d.pop(         d.setdefault(  d.values(
d.copy(        d.get(         d.keys(        d.popitem(     d.update(
>>> d['x'] = 3
>>> d
{'x': 3, 'y': 2}

They don't have the analog of push!, but in Julia push!(d, k => v) is (currently) equivalent to d[k] = v.

[torfjelde]

Presumably this is only an issue with pre-existing keys. To me, it's not entirely obvious what the right answer is. Anyone else have any thoughts?

Exactly; I'm also not certain what's "correct" here.

I was somewhat surprised to find that push! was even implemented for AbstractDict, as I usually associate push! with the action of "pushing onto a stack", i.e. not making that much sense in the case of containers without ordering 🤷

[timholy]

It's basically to support adding k/v pairs without needing to destructure the pair. But if your mental model of push! is "to the end" then it's indeed confusing.

On balance I lean towards thinking the current behavior is consistent, but it would be fine to leave this open and see if anyone else chimes in.