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

fix boxplot behavior when only y values are provided #95

Merged
merged 4 commits into from
Sep 29, 2017

Conversation

ValdarT
Copy link
Contributor

@ValdarT ValdarT commented Sep 16, 2017

This fixes the behavior of boxplot() when user only provides y values. Related to discussion in #56

I hope it's not too much of a hack.

Some quick tests:

boxplot(rand(10))

boxplot(rand(10,2))

boxplot(["a" "b"], rand(10,2))

boxplot([:c :d], rand(10,2))

boxplot(3:4, rand(10,2))

boxplot([3 4], rand(10,2))

boxplot([0.5 4], rand(10,2))

boxplot(rand(["a", "b"], 20), rand(20))

boxplot([rand(["a", "b"], 20) rand(["c", "d"], 20)], rand(20,2))

boxplot([rand([:a, :b], 20) rand([:c, :d], 20)], rand(20,2))

Copy link
Member

@mkborregaard mkborregaard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks - I guess this could also be applied to violin? I suggest you take out the second bit though.

src/boxplot.jl Outdated
if typeof(x) <: UnitRange
x = [getindex(x, d[:series_plotindex])]
end
# x values need to be categories
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

x values do not need to be categories - x values can be used to specify the position on the x axis as well.

@@ -5,6 +5,14 @@
notch_width(q2, q4, N) = 1.58 * (q4-q2)/sqrt(N)

@recipe function f(::Type{Val{:boxplot}}, x, y, z; notch=false, range=1.5, outliers=true, whisker_width=:match)
# if only y is provided, then x will be UnitRange 1:length(y)
if typeof(x) <: UnitRange
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@ValdarT
Copy link
Contributor Author

ValdarT commented Sep 16, 2017

Thanks for the quick reply! Sure, makes sense. Applied the changes.

@mkborregaard mkborregaard merged commit 52c835b into JuliaPlots:master Sep 29, 2017
@mkborregaard
Copy link
Member

Thanks a lot @ValdarT !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants