Repository: arviz-devs/arviz

Issue:
Posterior plot errors with boolean array
When passed a boolean array, `plot_posterior` fails with the following error because `np.histogram` assumes that subtraction is defined on the array:
```python
TypeError('numpy boolean subtract, the `-` operator, is not supported, use the bitwise_xor, the `^` operator, or the logical_xor function instead.')
```

This example reproduces the error:

```python
import numpy as np
import arviz as az

data = np.random.choice(a=[False, True], size=(4, 100))
az.plot_posterior(data)
```

Discussion:
utkarsh-maheshwari: Yes, it looks incomplete without the y axis. Don't we need it in the case of integers? 

Achieved this. 
![image](https://user-images.githubusercontent.com/32706356/119650576-9c568680-be41-11eb-91fd-f78821e9b47b.png)

through this:
```
import numpy as np
import arviz as az

data = np.random.choice(a=[False, True], size=(4, 100))
az.plot_posterior(data)
```
utkarsh-maheshwari: @OriolAbril Can the dtype of values be other than f, i or b?
sethaxen: > @OriolAbril Can the dtype of values be other than f, i or b?

That's a good question. In principle a PPL could produces samples of strings or objects, though all I am aware of currently would require first assigning integers to the strings/objects and then produce integer samplers. Do we anywhere in arviz assert that the dtype is amount these types?

> We may also want to update a bit more the apperance of the plot? @sethaxen? i.e. showing the y axis in the boolean case, or hiding the mean and annotating both bars with `count (xx%)` on top of them, things like that

Yes, I think it makes sense to label the bars with count and percent instead mean, and if this is done, an axis is not necessary.
I like the gap between the bins.
ahartikainen: True/False is an edge case from category dtype with 2 categories.

If we have a flag for category, then we could do this kind of (bar) plot for multiple categories too.
utkarsh-maheshwari: > If we have a flag for category, then we could do this kind of (bar) plot for multiple categories too.

So, does that mean 
if dtype == " f " -> kde
elif dtype == " i " -> hist
else -> bar 
OriolAbril: > So, does that mean
if dtype == " f " -> kde
elif dtype == " i " -> hist
else -> bar

Sorry about the delay, I think this is a good starting point, we can then improve/refactor the else case to handle the category case too, but before that we have to decide on an api, have this info in inferencedata or as an argument or both which will probably take some time. 
codecov[bot]: # [Codecov](https://codecov.io/gh/arviz-devs/arviz/pull/1707?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arviz-devs) Report
> Merging [#1707](https://codecov.io/gh/arviz-devs/arviz/pull/1707?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arviz-devs) (99aa489) into [main](https://codecov.io/gh/arviz-devs/arviz/commit/4d9caca88a9a3b4043062609cb9a401e3f08804d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arviz-devs) (4d9caca) will **decrease** coverage by `0.03%`.
> The diff coverage is `54.54%`.

[![Impacted file tree graph](https://codecov.io/gh/arviz-devs/arviz/pull/1707/graphs/tree.svg?width=650&height=150&src=pr&token=KZTdQCEf7t&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arviz-devs)](https://codecov.io/gh/arviz-devs/arviz/pull/1707?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arviz-devs)

```diff
@@            Coverage Diff             @@
##             main    #1707      +/-   ##
==========================================
- Coverage   90.86%   90.82%   -0.04%     
==========================================
  Files         108      108              
  Lines       11821    11826       +5     
==========================================
  Hits        10741    10741              
- Misses       1080     1085       +5     
```


| [Impacted Files](https://codecov.io/gh/arviz-devs/arviz/pull/1707?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arviz-devs) | Coverage Δ | |
|---|---|---|
| [arviz/plots/backends/matplotlib/posteriorplot.py](https://codecov.io/gh/arviz-devs/arviz/pull/1707/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arviz-devs#diff-YXJ2aXovcGxvdHMvYmFja2VuZHMvbWF0cGxvdGxpYi9wb3N0ZXJpb3JwbG90LnB5) | `94.21% <54.54%> (-4.07%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/arviz-devs/arviz/pull/1707?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arviz-devs).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arviz-devs)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/arviz-devs/arviz/pull/1707?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arviz-devs). Last update [4d9caca...99aa489](https://codecov.io/gh/arviz-devs/arviz/pull/1707?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arviz-devs). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arviz-devs).

utkarsh-maheshwari: `Added lines #L332 - L336 were not covered by tests ` 
How to resolve this error?
ahartikainen: We need to add test that has boolean data. (check current tests and see if you can add boolean data as input for the test for this plot)
utkarsh-maheshwari: I think I need to create a function `test_plot_posterior_boolean` below https://github.com/arviz-devs/arviz/blob/4d9caca88a9a3b4043062609cb9a401e3f08804d/arviz/tests/base_tests/test_plots_matplotlib.py#L946.
to test with a boolean data. Right?
ahartikainen: yes
utkarsh-maheshwari: I think now there are some errors not related to my code changes. 
utkarsh-maheshwari: I have no idea why tests are failing. It works fine when I write the same test in notebook.
Is there a better way to debug tests?
OriolAbril: > Is there a better way to debug tests?

Were tests passing locally? Whenever I get stuck on one or a couple tests I run them locally, and only those so I can iterate fast. pytest has a lot of features to select which subset of tests to run: https://docs.pytest.org/en/6.2.x/reference.html#command-line-flags. To begin with you have to call `pytest dir_or_file` where `dir_or_file` can be a path to a folder, a path to a file or a glob pattern, you can also use special syntax after that with colons to select which class/test function to run but I find that very complicated and always use the `-k` flag to filter tests that fulfill a condition, in your case it could be something like `-k "posterior and boolean"` to run only the specific test you added
utkarsh-maheshwari: @OriolAbril This is much better approach. Thanks!
utkarsh-maheshwari: Unable to understand why a codecov shows a lot of code becomes uncovered through these changes. It shows whole matplotlib posterior plot function is becomes uncovered!
OriolAbril: > Unable to understand why a codecov shows a lot of code becomes uncovered through these changes. It shows whole matplotlib posterior plot function is becomes uncovered!

If the azure job fails, nothing is uploaded to codecov, thus if all jobs fail, the coverage is 0

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f833db1..dd27e8d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -25,6 +25,7 @@
 * Fix R2 implementation ([1666](https://github.com/arviz-devs/arviz/pull/1666))
 * Added warning message in `plot_dist_comparison()` in case subplots go over the limit ([1688](https://github.com/arviz-devs/arviz/pull/1688))
 * Fix coord value ignoring for default dims ([2001](https://github.com/arviz-devs/arviz/pull/2001))
+* Fixed plot_posterior with boolean data ([1707](https://github.com/arviz-devs/arviz/pull/1707))
 
 ### Deprecation
 
diff --git a/arviz/plots/backends/bokeh/posteriorplot.py b/arviz/plots/backends/bokeh/posteriorplot.py
index 8f129b7..b1a2f16 100644
--- a/arviz/plots/backends/bokeh/posteriorplot.py
+++ b/arviz/plots/backends/bokeh/posteriorplot.py
@@ -286,18 +286,30 @@ def _plot_posterior_op(
             show=False,
         )
         _, hist, edges = histogram(values, bins="auto")
-    else:
+    elif values.dtype.kind == "i" or (values.dtype.kind == "f" and kind == "hist"):
         if bins is None:
-            if values.dtype.kind == "i":
-                bins = get_bins(values)
-            else:
-                bins = "auto"
+            bins = get_bins(values)
         kwargs.setdefault("align", "left")
         kwargs.setdefault("color", "blue")
         _, hist, edges = histogram(values, bins=bins)
         ax.quad(
             top=hist, bottom=0, left=edges[:-1], right=edges[1:], fill_alpha=0.35, line_alpha=0.35
         )
+    elif values.dtype.kind == "b":
+        if bins is None:
+            bins = "auto"
+        kwargs.setdefault("color", "blue")
+
+        hist = np.array([(~values).sum(), values.sum()])
+        edges = np.array([-0.5, 0.5, 1.5])
+        ax.quad(
+            top=hist, bottom=0, left=edges[:-1], right=edges[1:], fill_alpha=0.35, line_alpha=0.35
+        )
+        hdi_prob = "hide"
+        ax.xaxis.ticker = [0, 1]
+        ax.xaxis.major_label_overrides = {0: "False", 1: "True"}
+    else:
+        raise TypeError("Values must be float, integer or boolean")
 
     format_axes()
     max_data = hist.max()
diff --git a/arviz/plots/backends/matplotlib/posteriorplot.py b/arviz/plots/backends/matplotlib/posteriorplot.py
index 039e3d9..c7c7697 100644
--- a/arviz/plots/backends/matplotlib/posteriorplot.py
+++ b/arviz/plots/backends/matplotlib/posteriorplot.py
@@ -319,18 +319,23 @@ def _plot_posterior_op(
             rug=False,
             show=False,
         )
-    else:
+    elif values.dtype.kind == "i" or (values.dtype.kind == "f" and kind == "hist"):
         if bins is None:
-            if values.dtype.kind == "i":
-                xmin = values.min()
-                xmax = values.max()
-                bins = get_bins(values)
-                ax.set_xlim(xmin - 0.5, xmax + 0.5)
-            else:
-                bins = "auto"
+            xmin = values.min()
+            xmax = values.max()
+            bins = get_bins(values)
+            ax.set_xlim(xmin - 0.5, xmax + 0.5)
         kwargs.setdefault("align", "left")
         kwargs.setdefault("color", "C0")
         ax.hist(values, bins=bins, alpha=0.35, **kwargs)
+    elif values.dtype.kind == "b":
+        if bins is None:
+            bins = "auto"
+        kwargs.setdefault("color", "C0")
+        ax.bar(["False", "True"], [(~values).sum(), values.sum()], alpha=0.35, **kwargs)
+        hdi_prob = "hide"
+    else:
+        raise TypeError("Values must be float, integer or boolean")
 
     plot_height = ax.get_ylim()[1]
 


Test patch:
diff --git a/arviz/tests/base_tests/test_plots_bokeh.py b/arviz/tests/base_tests/test_plots_bokeh.py
index 329e898..3fa7972 100644
--- a/arviz/tests/base_tests/test_plots_bokeh.py
+++ b/arviz/tests/base_tests/test_plots_bokeh.py
@@ -988,6 +988,17 @@ def test_plot_posterior_discrete(discrete_model, kwargs):
     assert axes.shape
 
 
+def test_plot_posterior_boolean():
+    data = np.random.choice(a=[False, True], size=(4, 100))
+    axes = plot_posterior(data, backend="bokeh", show=False)
+    assert axes.shape
+
+
+def test_plot_posterior_bad_type():
+    with pytest.raises(TypeError):
+        plot_posterior(np.array(["a", "b", "c"]), backend="bokeh", show=False)
+
+
 def test_plot_posterior_bad(models):
     with pytest.raises(ValueError):
         plot_posterior(models.model_1, backend="bokeh", show=False, rope="bad_value")
diff --git a/arviz/tests/base_tests/test_plots_matplotlib.py b/arviz/tests/base_tests/test_plots_matplotlib.py
index 235fdb6..91c3b49 100644
--- a/arviz/tests/base_tests/test_plots_matplotlib.py
+++ b/arviz/tests/base_tests/test_plots_matplotlib.py
@@ -1025,12 +1025,26 @@ def test_plot_posterior(models, kwargs):
         assert axes.shape
 
 
+def test_plot_posterior_boolean():
+    data = np.random.choice(a=[False, True], size=(4, 100))
+    axes = plot_posterior(data)
+    assert axes
+    plt.draw()
+    labels = [label.get_text() for label in axes.get_xticklabels()]
+    assert all(item in labels for item in ("True", "False"))
+
+
 @pytest.mark.parametrize("kwargs", [{}, {"point_estimate": "mode"}, {"bins": None, "kind": "hist"}])
 def test_plot_posterior_discrete(discrete_model, kwargs):
     axes = plot_posterior(discrete_model, **kwargs)
     assert axes.shape
 
 
+def test_plot_posterior_bad_type():
+    with pytest.raises(TypeError):
+        plot_posterior(np.array(["a", "b", "c"]))
+
+
 def test_plot_posterior_bad(models):
     with pytest.raises(ValueError):
         plot_posterior(models.model_1, rope="bad_value")