Open ShuhuaGao opened 4 years ago
Can you please point out in the codebase where you think display
is being defined? It is strange that you are getting a print issue when we don't define any show method in the codebase, as far as I remember.
Can you please point out in the codebase where you think
display
is being defined? It is strange that you are getting a print issue when we don't define any show method in the codebase, as far as I remember.
Indeed no show
method is defined. That is why the error is caused. Could you reproduce the error mentioned above?
Given an object x
, Julia provides a default and proper implementation for show(x)
and thus print(x)
. However, REPL and IJulia outputs an object with display(x)
by default (i.e., with neither manual call of print
etc. nor semicolon termination) For text output (which is true here), display(x)
falls back to show(stdout, "text/plain", x)
(see doc here).
Since no method show(stdout, "text/plain", x::LearnBase.IntervalSet)
has been defined, the default dispatch of Julia seems to not work well and leads to the above errors. According to the error information, the default dispatch tries to call iterate
on LearnBase.IntervalSet
. The call of iterate
is attributed to the subtyping of AbstractSet
. In Julia codebase, there is (source)
function show(io::IO, ::MIME"text/plain", t::AbstractSet{T}) where T
The object t
will be iterated inside the above function. Unfortunately, the Base.iterate
method is not provided for LearnBase.DiscreteSet
(and, e.g., LearnBase.IntervalSet
). The key is that Julia provides no default implementation of Base.iterate
for AbstractSet
.
Overall, the issue can be easily fixed by providing show(stdout, "text/plain", x::LearnBase.DiscreteSet)
or dispatching Base.iterate
for DiscreteSet
(and other similar types).
I can reproduce the issue, thanks for reporting. Do you mind submitting a PR with the show
methods implemented? I am not following the AbstractSet
concept in Base, nor these set types defined in LearnBase. It would also be nice to know where they are being used other than Reinforce.jl. We need to update LearnBase and continue with the clean up. I've been trying to find the time to work on it, but the GeoStats.jl stack is really consuming a lot of my time.
I encountered the issue when playing with Reinforce.jl, and the essential reason is in
LearnBase
.Issue description: When a concrete type in
LearnBase
derived fromAbstractSet
is displayed automatically in REPL or IJulia (i.e., with no semicolon at the end), an error will be caused like follows:How to reproduce
Note that, if you suppress the output with a semicolon and then print it manually with
print(ds)
, then no error happens and the printed result isLearnBase.DiscreteSet{Array{Int64,1}}([1, 2, 3])
.Reason of the error The reason is that when a variable is displayed automatically in REPL or IJulia, the
display
function is used. That is, if you print the output withdisplay(ds)
, the same error is induced. It seems that, for subtypes ofAbstractSet
, the defaultdisplay
method tries to iterate over each element. However, there is no default implementation in Julia to iterate anAbstractSet
. (see documentation)Possible fix Two obvious fixes are possible
Add
Base.iterate
method for each related type inLearnBase
. Example: if we dispatchBase.iterate
forDiscreteSet
by iteratingDiscreteSet.items
, the displayed output of the aboveds
isHowever, an iteration method may make little sense for
LearnBase.IntervalSet
.Support
display
by implementing the MIMEshow
method for relevant types. (see documentation) Example:will display a
LearnBase.IntervalSet(-1.0, 1.0)
asMy suggestion is that
AbstractSet
pertaining to this issue.DiscreteSet
), implement alsoBase.iterate
. Another benefit is that, with iteration support, those types can be used in a for loop naturally.I can make a PR if you think the above suggestion is reasonable.