Fixes compatibility with StatsBase 0.4

[racinmat]

Changes needed in order to have #45 working.
Locally it works, tried with MLLabelUtils fork JuliaML/MLLabelUtils.jl#40.
I hope it's correct, all tests are passing.

[racinmat]

@johnnychen94

[johnnychen94]

Are these unexported by LearnBase 0.4?

DataView, AbstractObsView, AbstractBatchView, DataIterator, AbstractDataIterator, ObsIterator, BatchIterator

[racinmat]

AFAIK yes

julia> using LearnBase
julia> DataView
ERROR: UndefVarError: DataView not defined

julia> AbstractObsView
ERROR: UndefVarError: AbstractObsView not defined

julia> AbstractBatchView
ERROR: UndefVarError: AbstractBatchView not defined

julia> DataIterator
ERROR: UndefVarError: DataIterator not defined

julia> AbstractDataIterator
ERROR: UndefVarError: AbstractDataIterator not defined

julia> ObsIterator
ERROR: UndefVarError: ObsIterator not defined

julia> BatchIterator
ERROR: UndefVarError: BatchIterator not defined

julia> LearnBase.BatchIterator
LearnBase.BatchIterator

[racinmat]

There was quite a lot of unexporting in JuliaML/LearnBase.jl@v0.3.0...v0.4.0

[darsnack]

I can export these in JuliaML/LearnBase.jl#44 if that is the preferred end result. Seems like a lot got lost in translation between 0.3 and 0.4.

[johnnychen94]

Good to merge as long as the test passes

[johnnychen94]

But keep in mind that once JuliaML/LearnBase.jl#44 is merged, we will need to handle 0.5 compatibilities here, and that would be way more complex since the whole ObsDim trick is removed.

[racinmat]

Yes, I'm aware of that, thanks. But hopefully the interface will be simpler that, using dims the cleaner way.

[darsnack]

I need to be diligent about making sure everything still works, but on my local copy, I was able to replace all the ObsDim tricks and reduce the codebase.

[racinmat]

It would be great if you shared the diff, I think some small "migration guide" would be helpful given the fact that multiple libraries will be affected

[darsnack]

Absolutely, I just need to do a little clean-up since it's a mess of commenting out parts and deleting parts right now. But I'll share something soon.

[darsnack]

Here is a PR that reflects the diff. I also included a summary of my experience.

[racinmat]

Hm, there are some problems with dependencies, most probably related to ReferenceTests and its image related functions for Julia 1.0, I have no idea how to fix it. Maybe restrict version of ReferenceTests?

[darsnack]

Yeah I am not surprised. I didn't commit Project.toml because I have local copies of LearnBase, MLLabelUtils, and MLDataPattern floating around. I ran into a bunch of issues trying to get those dependencies to work together without version issues or method errors once I deprecated ObsDim. I'll try and clean up the dependency mess.

[johnnychen94]

Hm, there are some problems with dependencies, most probably related to ReferenceTests and its image related functions for Julia 1.0, I have no idea how to fix it. Maybe restrict version of ReferenceTests?

Should be fixed after JuliaRegistries/General#23858 (Edit: fixed)

There are some io changes in the nightly test, could you help to fix that as well? The reference tests could be relaxed using by keyword, for example: https://github.com/JuliaTesting/ReferenceTests.jl/pull/66/files#diff-818dc64cdb7fb1b6ec7e95962534fd7eb7a454391b62b881cd9ac521ea581b76R83-R84, or by simply strip the data summary part, e.g, https://github.com/JuliaTesting/ReferenceTests.jl/blob/c8104a1628295d9e2ce79f58a29a97f0c3dc26bf/test/runtests.jl#L58

[racinmat]

I see that in nightly, the string representation of type is different,
e.g. it returns

julia> typeof(zeros(2,3))
Matrix{Float64} (alias for Array{Float64, 2})

instead of

julia> typeof(zeros(2,3))
Array{Float64,2}

this should be also handled by custom comparison function?

[johnnychen94]

Yes, for example, in

MLDataPattern.jl/test/tst_datasubset.jl

Line 50 in 2cf8d0e

@test_reference "references/DataSubset2.txt" @io2str showcompact(::IO, DataSubset(X))

+ strip_summary(content::String) = join(split(content, "\n")[2:end], "\n")

- @test_reference "references/DataSubset2.txt" @io2str showcompact(::IO, DataSubset(X))
+ @test_reference "references/DataSubset2.txt" strip_summary(@io2str(showcompact(::IO, DataSubset(X))))

Such an example can be found in https://github.com/JuliaTesting/ReferenceTests.jl/blob/c8104a1628295d9e2ce79f58a29a97f0c3dc26bf/test/runtests.jl#L58

[racinmat]

I see, thanks. Done.

[johnnychen94]

lol the reference files in test/references might also need some update.

[racinmat]

And what exactly? Because they are passing on Julia 1.0 and 1, right?
In the end I used different comparison function so I don't have to call show explicitly in the @test_reference "references/DataSubset1.txt" DataSubset(X, Int64(1):Int64(nobs(X))) testcase

[johnnychen94]

@racinmat Thanks for updating the codes!