-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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] allow sorting Dict/Set values in show #33744
base: master
Are you sure you want to change the base?
Conversation
This may be helpful for regression test outputs that test on the display output, as we do for NarrativeTest.jl. It'd be nice to also have a flag to sort them headless operation (e.g. test harness output). |
|
||
if sorted | ||
try # sorting fails when elements are not comparable, collect can fail too | ||
t = sort!(collect(t)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can have a lazy dict type that maps all typemin(Int):typemax(Int)
to something. Materializing this as a vector is quite dangerous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Maybe the :sorted
attribute should get an Int
instead of a Bool
, serving as a threshold for the maximum size of the dict below which it's sorted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another possibility is a dictionary variant of https://github.com/JuliaArrays/MappedArrays.jl where the mapping function is super slow. It is perhaps better to use a variant of Iterators.take
that takes a timeout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again good point! I'm starting to wonder whether this is really feasable in the general case...
I think what we really ought to do here is switch to an ordered Dict implementation, which would decouple the output from the vagries of the internal hashing. |
This would solve some use-cases but not the one where one wishes to see the keys in sorted order (as opposed to in insertion order). |
This is wise; could it happen before the next LTS?. This will make regression testing and working with JSON data more predictable. Order preserving dictionaries have become the norm the least few years; since Python version 3.5, standard |
@clarkevans Maybe you can try Dictionaries.jl? It has Python-inspired hash dict implementation andyferris/Dictionaries.jl#13. It works with 1.0 already. Using this in the wild is important if we are going to have it in |
Even though @StefanKarpinski states:
and:
it seems it will not happen (with unordered Swiss Dict/table merged). I see Go's map is unordered, so ordered is not strictly the norm, but it's however randomized order. So, this PR is still relevant if ordered is not going to happen, as I proposed, but I'm now a bit conflicted with Swiss that much faster: https://brianlovin.com/hn/29848295
To compare with Go: |
It can clearly be convenient to have a dict or set printed in sorted order, but doing so by default would give the wrong idea about the order of iteration. Cf. e.g. #7153 for some discussion. But wouldn't it be great to have an opt-in way? With this PR together with #29249, you can!
Needs to be updated for other
show
methods, and forSet
, but I wanted to check first whether this has a chance before putting more work.