register in METADATA - Githubissues

tbreloff commented 8 years ago

Needs some good tests (use VisualRegressionTests?)... anything else holding this up?

Evizero commented 8 years ago

A sensible readme and some good tests, I would say

tbreloff commented 8 years ago

Trying to get to a semi-useful, semi-stable, and tested state for MLPlots. To that end, I added a testing framework with tests for corrplot, OnlineAI nets, and ROCAnalysis. @Evizero could you suggest a good example/tests for LearnBase and ValueHistories? It should be super easy to add yourself, if you want:

Add a section to test/runtests.jl, similar to https://github.com/JuliaML/MLPlots.jl/blob/master/test/runtests.jl#L74-L78
Add the relevant "using" command
Make sure the package (and any dependencies) are available for travis... that means adding to test/REQUIRE if its a registered package, or adding appropriate line(s) to test/travis_commands.jl
run include(Pkg.dir("MLPlots","test","runtests.jl")) from the REPL... it will run the tests, and ask you to replace the reference image if it doesn't match or if there isn't one yet (this uses VisualRegressionTests.jl)
consider adding the example to the readme, similar to OnlineAI for example
add the files and submit a PR

This seems like a lot of steps but it's actually pretty straightforward, and then we get robust visual tests and a nice looking readme :+1:

Evizero commented 8 years ago

I shall take a look. Thanks for sketching this out

Evizero commented 8 years ago

I am having problems on my macbook with displaying of unicode characters in the plots.

Maybe instead of doing complicated merges I will propose my (locally working) additions here

facts("LearnBase") do
    @plottest "Margin-based Loss" begin
        plot(LossFunctions.HingeLoss())
    end
    @plottest "Multiple Margin-based Losses - single plot" begin
        plot([ZeroOneLoss(), L1HingeLoss(), L2HingeLoss(), LogitMarginLoss()])
    end
    @plottest "Multiple Margin-based Losses - subplots" begin
        subplot([ZeroOneLoss(), L1HingeLoss(), L2HingeLoss(), LogitMarginLoss()])
    end
    @plottest "Distance-based Loss" begin
        plot(LossFunctions.L2DistLoss())
    end
    @plottest "Multiple Distance-based Losses - single plot" begin
        plot([L2DistLoss(), L1DistLoss(), EpsilonInsLoss(.4), LogitDistLoss()])
    end
    @plottest "Multiple Distance-based Losses - subplot" begin
        subplot([L2DistLoss(), L1DistLoss(), EpsilonInsLoss(.4), LogitDistLoss()])
    end
end

Evizero commented 8 years ago

This also raises a question: how detailed do we want the tests to be. They do take up space after all. maybe we should reduce the image size even further? This way we can do more involved tests. For example my snipped above does not test calling plot with explicit x-range like plot(myloss, -1, 1)

tbreloff commented 8 years ago

This is great... I'm happy to include these myself if you want.

Also I just pushed up something for ValueHistories... let me know if you think that's ok.

how detailed do we want the tests to be

I think just enough that we know the core functionality works. If we can test many features/options in a single plot, all the better.

Evizero commented 8 years ago

This is great... I'm happy to include these myself if you want.

yes please do.

Also I just pushed up something for ValueHistories... let me know if you think that's ok

Yes I saw, looks great. I will go through the code and see if there is one or two more testcases I can think of and also post them here

Evizero commented 8 years ago

my suggestion for value histories (i changed your test a little bit to make sure illegal types are ignored)

facts("ValueHistories") do
    @plottest "valuehistories" begin
        history = ValueHistories.DynMultivalueHistory()
        for i=1:100
            x = 0.1i
            push!(history, :a, x, sin(x))
            push!(history, :wrongtype, x, "$(sin(x))")
            if i % 10 == 0
                push!(history, :b, x, cos(x))
            end
        end
        plot(history)
    end

    @plottest "QueueUnivalueHistory" begin
        history = ValueHistories.QueueUnivalueHistory(Int)
        for i = 1:100
            push!(history, i, 2i)
        end
        plot(history)
    end

    @plottest "VectorUnivalueHistory" begin
        history = ValueHistories.VectorUnivalueHistory(Int)
        for i = 1:100
            push!(history, i, 100-i)
        end
        plot(history)
    end
end

tbreloff commented 8 years ago

Thanks! I'll add these, then prepare to register.

Evizero commented 8 years ago

The tests would need the changes suggested here (Dict to KW) to work: https://github.com/JuliaML/MLPlots.jl/commit/e098c8958497c9cec3c3ea0abecf540490bd9cf8#commitcomment-17339934

Evizero commented 8 years ago

Thanks! I'll add these, then prepare to register.

Thank you. Yes I agree that with these basic tests in place registering is a good next step. That is a nice plot-testing framework that you wrote there.

tbreloff commented 8 years ago

I'm having trouble with the unicode symbols, as you mentioned. I'm not sure what the right solution is...

Evizero commented 8 years ago

maybe its a matplot lib thing? well if travis agrees on the same issue then I think we are fine

tbreloff commented 8 years ago

Check out the readme... let me know if you're happy

Evizero commented 8 years ago

I made some additions to the readme to add a little context, can you think of a couple sentences for OnlineAI and Corplot similar to what I did for my packages?

tbreloff commented 8 years ago

I generally like what you did with the readme. I'll add to it today.

On Wednesday, May 4, 2016, Christof Stocker notifications@github.com wrote:

I made some additions to the readme to add a little context, can you think of a couple sentences for OnlineAI and Corplot similar to what I did for my packages?

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/JuliaML/MLPlots.jl/issues/9#issuecomment-216784093

JuliaML / MLPlots.jl

register in METADATA #9