-
-
Notifications
You must be signed in to change notification settings - Fork 306
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
add raincloud plots from Beacon Biosignals #1725
Conversation
@haberdashPI mentioned this in a previous discussion that we've had. I wanted to repost it here so others can see and so that we potentially make this change as well.
|
Compile Times benchmarkNote, that these numbers may fluctuate on the CI servers, so take them with a grain of salt. using time
This PR does not change the using time. ttfp time
This PR does not change the ttfp time. |
* support flat array style for raincloud plot * hacky fix * revise category labels * fix labels * fix x labels again * working dodge / color * update docs * update news/docs * moved news item * fix `rainclouds` news item
Sorry about that, it looks like my use of some newer keyword syntax is leading to the failures on 1.3: |
Co-authored-by: David Little <[email protected]>
Co-authored-by: David Little <[email protected]>
Ok, this should build now... |
It should probably be cycling color; I can investigate/debug today or tomorrow. Is there anything special I should know to setup the doc build on my machine? |
I think it was doing that before, if I'm not mistaken.. So think something got removed ;)
I usually just copy out the code blocks, since the setup is a bit annoying and you can't just build a single page 😅 |
I've made some updates to the docs in #1936 . This also fixes a few bugs I found in the process. |
* improve raincloud examples * improve docs * fix hist coloring * remove redundant computations
Test failures due to: #1937 |
#### | ||
|
||
function mockup_distribution(N) | ||
all_possible_labels = ["Single Mode", "Double Mode", "Single Mode", "Double Mode", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it intentional that single mode and double mode show up twice here?
random_mean = rand_localized(0, 8) | ||
random_spread_coef = rand_localized(0.3, 1) | ||
data_points = random_spread_coef*randn(Int(round(N/2.0))) .+ random_mean | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
random_mean = rand_localized(0, 8) | |
random_spread_coef = rand_localized(0.3, 1) | |
data_points = random_spread_coef*randn(Int(round(N/2.0))) .+ random_mean | |
potential duplication? otherwise these variables get overridden immediately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks right to me. (Note the vcat
on line 33).
Co-authored-by: Hannah Robertson <[email protected]>
What's left to have this merge? I thought |
Yes, I think that's what's mostly left. |
I think this is addressed? At least when I ran the help examples on my machine they are colored now. (#1936) I'll set up a PR to fix @hannahilea's comments. |
Ah you're right, great :) Somehow the last time I checked the docs build, it was still all black |
#2019 makes a small fix to address @hannahilea's comments. |
* small fix to raincloud docs * fix modes
Yay, thanks everyone :) |
@@ -249,7 +249,7 @@ function plot!(plot::RainClouds) | |||
show_median=show_median, side=side, width=width_ratio*cloud_width, plot.cycle, | |||
plot.color, gap=0) | |||
elseif clouds === hist | |||
for (_, ixs) in group_labels(zip(category_labels, plot.dodge[]), data_array) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just realize that this was actually intentional. Without this, the histogram doesn't group by dodge, and so you get one histogram for all of the dodge groups. 😢
I'm submitting a small PR soon to implement orientation=:horizontal
for rainclouds. I'll add a fix for this in that PR.
We decided to open source @kimlaberinto amazing raincloud plots and add them to base Makie.
This is a direct port of the docs & the latest implementation.
@kimlaberinto wanted to spent some more time to make the recipe even more usable/pretty before we merge.
We also need to add some tests to the reference image tests.
cc: @haberdashPI @ericphanson @mich11 @palday @jrevels