OrderedCollections.jl icon indicating copy to clipboard operation
OrderedCollections.jl copied to clipboard

Remove convert method to OrderedDict

Open fingolfin opened this issue 1 year ago • 2 comments

The generic one in Julia stdlib does the job, except for the deprecation warning shown when converting from an unordered dict (such as Base.Dict) to an OrderedDict{K,V} type.

This avoids potential invalidations in packages. There also is no explanation as to why it makes sense to forbids such conversions.

The depwarn was introduced in bea403157497de7821d824fe44d03f7aff38743e but I don't know why. I certainly don't want to hastily merge this without understanding this better. I am also happy to close this PR and instead open one which adds a comment with an explanation of why this conversion is deprecated, as soon as somebody explains it to me :-). Or just open a PR with that explanation and we merge that and close this PR.

CC @timholy @oxinabox who perhaps might have an idea?

Also I figure one should perhaps use nanosoldier to test a PR like this given how critical this package is in the ecosystem? but I have no idea how to do that (and maybe also not the access rights? I know too little about it)

fingolfin avatar Nov 24 '24 23:11 fingolfin

I'm guessing it's because the user is making that conversion because they care about the order in the resulting object, but if you're converting from an unordered container the resulting order is unpredictable? But honestly I don't know.

timholy avatar Nov 25 '24 08:11 timholy

I am fine with this.

It's not always sensible to convert an Unordered Dict to an OrderedDict, since the order of an unordered dict is arbitary. and caprecious even, since different dicts with same content can have different order, and even the order of a dict can change if you pass it over the network (https://github.com/JuliaLang/julia/issues/34265). but occationally you don't care about the order of the dictionary so long as it is consistent (like in the case you are passing it over a network), or you might want future additions to be kept in order but don't care for stuff you already have gotten unordered.

And given there is already a fallback that does the same thing (minus the warning messages) we do not have worry about breaking this. This is fine to delete.

oxinabox avatar Nov 26 '24 08:11 oxinabox