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

undesirable output when showing empty set in the REPL #45229

Open
StephenVavasis opened this issue May 8, 2022 · 13 comments
Open

undesirable output when showing empty set in the REPL #45229

StephenVavasis opened this issue May 8, 2022 · 13 comments
Labels
display and printing Aesthetics and correctness of printed representations of objects.

Comments

@StephenVavasis
Copy link
Contributor

I am currently working on SortedSet from the package DataStructures.jl. SortedSet does not have its own output routines; instead, it relies on the routines inherited from AbstractSet in Base. However, these routines have unexpected behavior for an empty set. (Julia 1.7.1 on Windows, see below). I confirmed that the REPL output for ordinary Set is also inconsistent when the set is empty, but in the case of ordinary Set the issue is less obvious.

julia> s = SortedSet{Int}([4,3,2,1])
SortedSet{Int64, Base.Order.ForwardOrdering} with 4 elements:
  1
  2
  3
  4

julia> s = SortedSet{Int}([])
SortedSet{Int64, Base.Order.ForwardOrdering}(DataStructures.BalancedTree23{Int64, Nothing, Base.Order.ForwardOrdering}(Base.Order.ForwardOrdering(), DataStructures.KDRec{Int64, Nothing}[DataStructures.KDRec{Int64, Nothing}(1, 2, nothing), DataStructures.KDRec{Int64, Nothing}(1, -1, nothing)], DataStructures.TreeNode{Int64}[DataStructures.TreeNode{Int64}(1, 2, 0, 0, 221796496, 31538)], 1, 1, Int64[], Int64[], BitSet([1, 2]), [224399536, 0, 0], [281825360, 0, 0]))
@Moelf
Copy link
Sponsor Contributor

Moelf commented May 8, 2022

it's just how things are displayed:

julia> []
Any[]

julia> [1]
1-element Vector{Int64}:
 1

you should open an issue with DataStructures.jl if you think it's too verbose, probably no actionable item here.

@StephenVavasis
Copy link
Contributor Author

Could you elaborate a bit more? Why is it a good idea to display 0-entry collections differently from collections with one or more entries?

@Moelf
Copy link
Sponsor Contributor

Moelf commented May 9, 2022

It doesn't matter, as long as it's readable and not confusing.

@StephenVavasis
Copy link
Contributor Author

But in this case (specialized for SortedSet), it's not readable and it is confusing. Methods written for AbstractSet or other abstract classes in Base should be reasonably flexible for library writers.

Can you tell me: what exactly is that big string that is in my original posting? What statement in Base produced that?

@Moelf
Copy link
Sponsor Contributor

Moelf commented May 9, 2022

But in this case (specialized for SortedSet), it's not readable and it is confusing.

thus my original suggestion: opening an issue in DataStructures.jl.

The solution to that is for DataStructures.jl to fix their Base.show() for SortedSet:
https://github.com/JuliaCollections/DataStructures.jl/blob/5ce4aa31fac243a1499aac71291bf48ce574d1cd/src/sorted_set.jl#L559

@StephenVavasis
Copy link
Contributor Author

I continue to think this should be fixed in Base. If functions associated with abstract classes in Base that needlessly cause problems for library writers, this somewhat defeats the purpose of even having abstract classes in Base.

But in any case, do you know which statement in Base generated that very long string in my original post? This might inform me what needs to be done with SortedSet.

@inkydragon
Copy link
Sponsor Member

Mabye update julia?

julia v1.7.2 + DataStructures v0.18.12

julia> s = SortedSet{Int}([4,3,2,1])
SortedSet{Int64, Base.Order.ForwardOrdering} with 4 elements:
  1
  2
  3
  4

julia> s = SortedSet{Int}([])
SortedSet(Int64[],
Base.Order.ForwardOrdering())

julia> SortedSet{Int}(Any[])
SortedSet(Int64[],
Base.Order.ForwardOrdering())

julia> SortedSet{Int}([])
SortedSet(Int64[],
Base.Order.ForwardOrdering())

julia> versioninfo()
Julia Version 1.7.2
Commit bf53498635 (2022-02-06 15:21 UTC)
Platform Info:
  OS: Windows (x86_64-w64-mingw32)
  CPU: Intel(R) Core(TM) i5-9400F CPU @ 2.90GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-12.0.1 (ORCJIT, skylake)

(@v1.7) pkg> st
      Status `C:\Users\woclass\.julia\environments\v1.7\Project.toml`
  [6e4b80f9] BenchmarkTools v1.2.2
  [864edb3b] DataStructures v0.18.12
  [31a5f54b] Debugger v0.6.8

@StephenVavasis
Copy link
Contributor Author

Here is additional backstory on this request. I am the original author and still the main maintainer of sorted container data structures. The bad behavior reported in my original post is not for the master branch of DataStructures.jl but rather for the version in pull request: JuliaCollections/DataStructures.jl#787. In fact, in this PR 787, I removed the show routine that I had originally written for SortedSet because it didn't work properly and also contributed to code bloat, slow loading, etc. My idea was to rely on the show for AbstractSet. And indeed, this works well except for the empty set, for which AbstractSet's show does something mysterious.

In any case, it would be helpful if someone could tell me where in Base is the code that outputs an AbstractSet to the REPL.

@ianatol
Copy link
Member

ianatol commented May 9, 2022

... it would be helpful if someone could tell me where in Base is the code that outputs an AbstractSet to the REPL

https://github.com/JuliaLang/julia/blob/master/base/show.jl#L148-L189

@StephenVavasis
Copy link
Contributor Author

OK, thanks! This helped me get started. What I found is: there are several classes in Base derived from AbstractSet including BitSet, Set, KeySet, and IdSet. Of these, none relies on show for AbstractSet except the unexported IdSet. So this means that the show method for AbstractSet is provided mainly for library writers, especially library writers for DataStructures.jl such as myself. And indeed, it is a quite useful method except in the empty-set case.

@JeffBezanson JeffBezanson added the display and printing Aesthetics and correctness of printed representations of objects. label May 10, 2022
@JeffBezanson
Copy link
Sponsor Member

This is due to #35417. There we decided to prefer 2-argument, parseable output for empty collections, but maybe that's the wrong default for non-arrays.

@StephenVavasis
Copy link
Contributor Author

Just for further information: the big string printed out by show for AbstractSet in my original post is not actually parsable to reconstruct the object, at least, not in the obvious way. Below, I tried to feed the output back into the REPL and received an error.

julia> SortedSet{Int}()
SortedSet{Int64, Base.Order.ForwardOrdering}(DataStructures.BalancedTree23{Int64, Nothing, Base.Order.ForwardOrdering}(Base.Order.ForwardOrdering(), DataStructures.KDRec{Int64, Nothing}[DataStructures.KDRec{Int64, Nothing}(1, 191898512, nothing), DataStructures.KDRec{Int64, Nothing}(1, 191898640, nothing)], DataStructures.TreeNode{Int64}[DataStructures.TreeNode{Int64}(1, 2, 0, 0, 12, 13)], 1, 1, Int64[], Int64[], BitSet([1, 2]), [32, 4, 10999411245313], [32, 4, 10999411245313]))

julia> SortedSet{Int64, Base.Order.ForwardOrdering}(DataStructures.BalancedTree23{Int64, Nothing, Base.Order.ForwardOrdering}(Base.Order.ForwardOrdering(), DataStructures.KDRec{Int64, Nothing}[DataStructures.KDRec{Int64, Nothing}(1, 191898512, nothing), DataStructures.KDRec{Int64, Nothing}(1, 191898640, nothing)], DataStructures.TreeNode{Int64}[DataStructures.TreeNode{Int64}(1, 2, 0, 0, 12, 13)], 1, 1, Int64[], Int64[], BitSet([1, 2]), [32, 4, 10999411245313], [32, 4, 10999411245313]))
ERROR: MethodError: no method matching DataStructures.BalancedTree23{Int64, Nothing, Base.Order.ForwardOrdering}(::Base.Order.ForwardOrdering, ::Vector{DataStructures.KDRec{Int64, Nothing}}, ::Vector{DataStructures.TreeNode{Int64}}, ::Int64, ::Int64, ::Vector{Int64}, ::Vector{Int64}, ::BitSet, ::Vector{Int64}, ::Vector{Int64})
Closest candidates are:
  DataStructures.BalancedTree23{K, D, Ord}(::Val{true}, ::Any, ::Ord, ::Bool) where {K, D, Ord<:Base.Order.Ordering} at c:\Users\vavasis\OneDrive - University of Waterloo\Julia_pkg_devdir\DataStructures.jl\src\balanced_tree.jl:989
  DataStructures.BalancedTree23{K, D, Ord}(::Ord) where {K, D, Ord<:Base.Order.Ordering} at c:\Users\vavasis\OneDrive - University of Waterloo\Julia_pkg_devdir\DataStructures.jl\src\balanced_tree.jl:134
Stacktrace:
 [1] top-level scope
   @ REPL[7]:1

@StephenVavasis
Copy link
Contributor Author

The development branch mentioned in my original post has now become the master branch for DataStructures.jl. So fixing the undesirable behavior reported in my original post is now slightly more pressing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display and printing Aesthetics and correctness of printed representations of objects.
Projects
None yet
Development

No branches or pull requests

5 participants