Add Queue `show` method to hide backing Deque

[jordancluts]

I was looking over the Queue method per #479 and while I didn't find any issues on that front (not that I'm in any way qualified to make that assessment) I noticed that during printing at the REPL the backing Deque is printed inside. Decided to try to write a small show method to hide the backing type to pretty it up. This formatting matches how a Deque is currently printed.

Changes the REPL representation from Queue [Deque [[6, 10]]] to Queue [[6, 10]]

[oxinabox]

A method for show should generally parse + eval to give back the original object. So we would want Queue([6, 10]) we may not have that right now, but we should.

Also we probably want to call show anything we are displaying (except strings) rather than calling print/interpolating it. Since that will mean than stuff like IOContext with showcompact gets passed on.

[jordancluts]

Sorry for causing probably more work than this is all worth. I like DataStructures.jl and wanted to help in some small way to polish things up for 1.0. I've never gotten an actual PR accepted before though so I'm figuring things out.

For the latter point would something more like the following be what you mean by calling calling "call show on anything we are displaying"?

function Base.show(io::IO, s::Queue)
       print(io,"Queue(")
       show(collect(s.store))
       print(io,")")
 end

As for the first point, a constructor to create a Queue from an array (or probably other iterables as well) so that Queue([6,10]) returns the same object is easy to implement but only if Deque has essentially the same functionality. I could look into seeing if I could implement something if it would be desirable.

If Deque were being modified it's show could be changed to have the same style. Deque([1,2])

Last point, should they be shown as Queue{Int}([6,10]) and Deque{Float64}([1.0,2.0]) if I were to undertake the effort?

[oxinabox]

Sorry for causing probably more work than this is all worth. I like DataStructures.jl and wanted to help in some small way to polish things up for 1.0. I've never gotten an actual PR accepted before though so I'm figuring things out.

No this is appreciated

For the latter point would something more like the following be what you mean by calling calling "call show on anything we are displaying"?

function Base.show(io::IO, s::Queue)
       print(io,"Queue(")
       show(io, collect(s.store))
       print(io,")")
end

(you missed passing the io to the show)

As for the first point, a constructor to create a Queue from an array (or probably other iterables as well) so that Queue([6,10]) returns the same object is easy to implement but only if Deque has essentially the same functionality. I could look into seeing if I could implement something if it would be desirable.

Does Queue and Deque not already have these? They should. basically all collections should have a constructor that accept an iterator. That should be a seperate PR though (that could be merged before this one)

If Deque were being modified it's show could be changed to have the same style. Deque([1,2])

yes, and that could be in this PR.

Last point, should they be shown as Queue{Int}([6,10]) and Deque{Float64}([1.0,2.0]) if I were to undertake the effort?

I think Queue([6,10]) and Deque([1.0,2.0]) is probably fine. they pull their type parameter from the backing collection. and show on that should print enough type information to recreate it.

[jordancluts]

Does Queue and Deque not already have these?

No they do not. The only way to construct one at the moment is empty (unless I am really missing something but I see no constructors for it and it errors at the REPL). Stacks similarly don't have printing or a constructor from an iterable.

I'll add the improved show methods to those 3 types in this PR but then we can pause this PR until the constructors are done.

I'm going to open an issue about the Deque (and Queue and Stack) constructors for tracking, then when I have time I'll see if I can figure out the implementation of that constructor in Deque. The corresponding constructors in Stack/Queue would then be simple.

[jordancluts]

One further question.

Deque implements a custom collect function which seems pointless since Deque already implements iteration and the implementation of the collect function doesn't appear to do anything meaningful to improve performance over whatever naive thing the default collect does.

As such should we show the contents of a Queue by simply collecting the Queue instead of collecting the backing Deque? i.e. using show(io,collect(s)) instead of show(io,collect(s.store)) (the results in the REPL are the same but I'm unsure of performance impacts although I can't imagine what ones there could be)

and should the Deque collect method be removed?

[oxinabox]

I am not sure, I haven't looked at the collect method. Maybe it shouldn't exist?