Suggested improvement to Histograms (grouped and non grouped)

[ibecav]

Hi,

While I applaud and appreciate Hadley's decision within ggplot2 to not automatically assign a number of bins in a histogram I do not believe that's the right decision for ggstatsplot where we're making some rational choices for the end user. It generates a warning message that many won't initially understand and clutters things up.

This fix provides a rational default value for bins if the user leaves it NULL. No impact if they specify something. The default is a common choice:

binwidth <- (max(data$x) - min(data$x))/sqrt(length(data$x))

I also removed some comment lines since at least in the build process for me it generates annoying warnings. They are completely non substantive.

Thanks for considering.

Chuck

[ibecav]

Sorry next time I'll rebase and squash and spare you so much detail.

[IndrajeetPatil]

@ibecav Thank you so much for the PR!

Can you please provide few reprex (https://github.com/tidyverse/reprex) showing that the behavior of both gghistostats and grouped_gghistostats will be backward compatible with these changes?
That makes it easy to review these changes. Thanks.

(Also, don't worry about the AppVeyor build failing. Binaries are yet to be built for ggcorrplot's new version.)

[ibecav]

Hi @IndrajeetPatil ,

So the easiest is simply by using your current example(s) like:

ggstatsplot::gghistostats(
     data = datasets::iris,
     x = Sepal.Length,
    bar.measure = "count",
    type = "p",
    bf.message = TRUE,
    caption = "Displaying results from both parametric and bayesian tests.",
    bf.prior = 0.8,
    test.value = 3,
    centrality.para = "mean",
    test.value.line = TRUE,
#    binwidth = 0.10,
    bar.fill = "grey50"
)

Original example

Commenting out binwidth

[ibecav]

Here's the grouped example

ggstatsplot::grouped_gghistostats(
    data = iris,
    x = Sepal.Length,
    test.value = 5,
    grouping.var = Species,
    bar.fill = "orange",
    nrow = 1,
    messages = FALSE
)
#> t is large; approximation invoked.
#> t is large; approximation invoked.

Created on 2018-09-14 by the reprex
package (v0.2.0).

[IndrajeetPatil]

Merged. Thanks a lot!

It was indeed a bit annoying to keep getting those binwidth related messages.

[ibecav]

My pleasure!

I very much appreciate the decision Hadley made in the core ggplot2 pkg but in this package it is just a stumbling block.

If you don't mind I'd like to continue to contribute as I can since I really like where you're headed with this package. Sometime in the next few days I'll make some suggestions on how to upadte the documentation for the changes you've merged already and then push on to some other ideas.

Chuck

[IndrajeetPatil]

Dear Chuck,

Thanks a lot! I'd very much appreciate your contributions!

One concrete thing I can really use some help with is this issue I haven't been able to solve:
#38

I feel like this is a bug, but not sure how to get rid of it.

[ibecav]

Hi Indrajeet, Thanks. I have a couple of things I want to look at in histograms but I’ll move to scatterplots next and see if anything comes to mind. Chuck Sent from my mobile please forgive my brevity

…

On Sep 15, 2018, at 10:03, Indrajeet Patil ***@***.***> wrote: Dear Chuck, Thanks a lot! I'd very much appreciate your contributions! One concrete thing I can really use some help with is this issue I haven't been able to solve: #38 I feel like this is a bug, but not sure how to get rid of it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.