fixes ggplot histogram to match R's version (#754)

ploomber · Jul 26, 2023 · 0233eb0 · 0233eb0
1 parent 0b7c03a
commit 0233eb0
Show file tree

Hide file tree

Showing 50 changed files with 27 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,7 +2,7 @@
 
 ## 0.9.0dev
 * [Fix] Fix error that was incorrectly converted into a print message
-* [API Change] A shorter name is now assigned as an alias by default
+* [Fix] Modified histogram query to ensure histogram binning is done correctly (#751)
 * [Fix] Fix bug that caused the `COMMIT` not to work when the SQLAlchemy driver did not support `set_isolation_level`
 * [Fix] Fixed vertical color breaks in histograms (#702)
 

diff --git a/doc/api/magic-plot.md b/doc/api/magic-plot.md
@@ -5,7 +5,7 @@ jupytext:
     extension: .md
     format_name: myst
     format_version: 0.13
-    jupytext_version: 1.14.6
+    jupytext_version: 1.14.7
 kernelspec:
   display_name: Python 3 (ipykernel)
   language: python
@@ -139,6 +139,12 @@ generate histograms without explicitly removing NULL entries.
 %sqlplot histogram --table penguins.csv --column body_mass_g 
 ```
 
+When plotting a histogram, it divides a range with the number of bins - 1 to calculate a bin size. Then, it applies round half down relative to the bin size and categorizes continuous values into bins to replicate right closed intervals from the ggplot histogram in R.
+
+![body_mass_g](../static/body_mass_g_R.png)
+
++++
+
 ### Number of bins
 
 ```{code-cell} ipython3

diff --git a/doc/static/body_mass_g_R.png b/doc/static/body_mass_g_R.png
diff --git a/src/sql/ggplot/ggplot.py b/src/sql/ggplot/ggplot.py
@@ -70,9 +70,11 @@ def _draw(self, other) -> mpl.figure.Figure:
                 ax_ = self.figure.add_subplot(n_rows, n_cols, i + 1)
                 facet_key_val = {"key": other.facet, "value": value[0]}
                 self.geom.draw(self, ax_, facet_key_val)
+                handles, labels = ax_.get_legend_handles_labels()
                 ax_.set_title(value[0])
                 ax_.tick_params(axis="both", labelsize=7)
-                ax_.legend(prop={"size": 10})
+                # reverses legend order so alphabetically first goes on top
+                ax_.legend(handles[::-1], labels[::-1], prop={"size": 10})
                 if other.legend is False:
                     plt.legend("", frameon=False)
                 self.axs.append(ax_)

diff --git a/src/sql/plot.py b/src/sql/plot.py
@@ -455,7 +455,9 @@ def histogram(
             bottom += values_
 
         ax.set_title(f"Histogram from {table!r}")
-        ax.legend()
+        # reverses legend order so alphabetically first goes on top
+        handles, labels = ax.get_legend_handles_labels()
+        ax.legend(handles[::-1], labels[::-1])
     elif isinstance(column, str):
         bin_, height, bin_size = _histogram(
             table, column, bins, with_=with_, conn=conn, facet=facet
@@ -534,12 +536,13 @@ def _histogram(table, column, bins, with_=None, conn=None, facet=None):
                 f"bins are '{bins}'. Please specify a valid number of bins."
             )
 
+        # Use bins - 1 instead of bins and round half down instead of floor
+        # to mimic right-closed histogram intervals in R ggplot
         range_ = max_ - min_
-        bin_size = range_ / bins
-
+        bin_size = range_ / (bins - 1)
         template_ = """
             select
-            floor("{{column}}"/{{bin_size}})*{{bin_size}} as bin,
+            ceiling("{{column}}"/{{bin_size}} - 0.5)*{{bin_size}} as bin,
             count(*) as count
             from "{{table}}"
             {{filter_query}}
@@ -595,9 +598,14 @@ def _histogram_stacked(
         conn = sql.connection.ConnectionManager.current
 
     cases = []
+    tolerance = bin_size / 1000  # Use to avoid floating point error
     for bin in bins:
-        case = f'SUM(CASE WHEN FLOOR({column}/{bin_size})*{bin_size} = {bin} \
-                 THEN 1 ELSE 0 END) AS "{bin}",'
+        # Use round half down instead of floor to mimic
+        # right-closed histogram intervals in R ggplot
+        case = (
+            f"SUM(CASE WHEN ABS(CEILING({column}/{bin_size} - 0.5)*{bin_size} "
+            f"- {bin}) <= {tolerance} THEN 1 ELSE 0 END) AS '{bin}',"
+        )
         cases.append(case)
 
     cases = " ".join(cases)
@@ -614,7 +622,8 @@ def _histogram_stacked(
         {{cases}}
         FROM "{{table}}"
         {{filter_query}}
-        GROUP BY {{category}};
+        GROUP BY {{category}}
+        ORDER BY {{category}} DESC;
         """
     )
     query = template.render(

diff --git a/src/tests/baseline_images/test_ggplot/facet_wrap_custom_fill.png b/src/tests/baseline_images/test_ggplot/facet_wrap_custom_fill.png
diff --git a/src/tests/baseline_images/test_ggplot/facet_wrap_custom_fill_and_color.png b/src/tests/baseline_images/test_ggplot/facet_wrap_custom_fill_and_color.png
diff --git a/src/tests/baseline_images/test_ggplot/facet_wrap_custom_stacked_histogram.png b/src/tests/baseline_images/test_ggplot/facet_wrap_custom_stacked_histogram.png
diff --git a/src/tests/baseline_images/test_ggplot/facet_wrap_custom_stacked_histogram_cmap.png b/src/tests/baseline_images/test_ggplot/facet_wrap_custom_stacked_histogram_cmap.png
diff --git a/src/tests/baseline_images/test_ggplot/facet_wrap_default.png b/src/tests/baseline_images/test_ggplot/facet_wrap_default.png
diff --git a/src/tests/baseline_images/test_ggplot/facet_wrap_default_no_legend.png b/src/tests/baseline_images/test_ggplot/facet_wrap_default_no_legend.png
diff --git a/src/tests/baseline_images/test_ggplot/facet_wrap_nulls_data.png b/src/tests/baseline_images/test_ggplot/facet_wrap_nulls_data.png
diff --git a/src/tests/baseline_images/test_ggplot/histogram_categorical_combined.png b/src/tests/baseline_images/test_ggplot/histogram_categorical_combined.png
diff --git a/src/tests/baseline_images/test_ggplot/histogram_custom_color.png b/src/tests/baseline_images/test_ggplot/histogram_custom_color.png
diff --git a/src/tests/baseline_images/test_ggplot/histogram_custom_fill.png b/src/tests/baseline_images/test_ggplot/histogram_custom_fill.png
diff --git a/src/tests/baseline_images/test_ggplot/histogram_custom_fill_and_color.png b/src/tests/baseline_images/test_ggplot/histogram_custom_fill_and_color.png
diff --git a/src/tests/baseline_images/test_ggplot/histogram_default.png b/src/tests/baseline_images/test_ggplot/histogram_default.png
diff --git a/src/tests/baseline_images/test_ggplot/histogram_numeric_categorical_combined.png b/src/tests/baseline_images/test_ggplot/histogram_numeric_categorical_combined.png
diff --git a/...eline_images/test_ggplot/histogram_numeric_categorical_combined_custom_fill.png b/...eline_images/test_ggplot/histogram_numeric_categorical_combined_custom_fill.png
diff --git a/...mages/test_ggplot/histogram_numeric_categorical_combined_custom_multi_color.png b/...mages/test_ggplot/histogram_numeric_categorical_combined_custom_multi_color.png
diff --git a/...images/test_ggplot/histogram_numeric_categorical_combined_custom_multi_fill.png b/...images/test_ggplot/histogram_numeric_categorical_combined_custom_multi_fill.png
diff --git a/src/tests/baseline_images/test_ggplot/histogram_stacked_custom_cmap.png b/src/tests/baseline_images/test_ggplot/histogram_stacked_custom_cmap.png
diff --git a/src/tests/baseline_images/test_ggplot/histogram_stacked_custom_color.png b/src/tests/baseline_images/test_ggplot/histogram_stacked_custom_color.png
diff --git a/src/tests/baseline_images/test_ggplot/histogram_stacked_custom_color_and_fill.png b/src/tests/baseline_images/test_ggplot/histogram_stacked_custom_color_and_fill.png
diff --git a/src/tests/baseline_images/test_ggplot/histogram_stacked_default.png b/src/tests/baseline_images/test_ggplot/histogram_stacked_default.png
diff --git a/src/tests/baseline_images/test_ggplot/histogram_stacked_large_bins.png b/src/tests/baseline_images/test_ggplot/histogram_stacked_large_bins.png
diff --git a/src/tests/baseline_images/test_ggplot/histogram_with_default.png b/src/tests/baseline_images/test_ggplot/histogram_with_default.png
diff --git a/src/tests/baseline_images/test_ggplot/test_facet_wrap_default_with_nulls.png b/src/tests/baseline_images/test_ggplot/test_facet_wrap_default_with_nulls.png
diff --git a/src/tests/baseline_images/test_magic_plot/hist.png b/src/tests/baseline_images/test_magic_plot/hist.png
diff --git a/src/tests/baseline_images/test_magic_plot/hist_bin.png b/src/tests/baseline_images/test_magic_plot/hist_bin.png
diff --git a/src/tests/baseline_images/test_magic_plot/hist_custom.png b/src/tests/baseline_images/test_magic_plot/hist_custom.png
diff --git a/src/tests/baseline_images/test_magic_plot/hist_null.png b/src/tests/baseline_images/test_magic_plot/hist_null.png
diff --git a/src/tests/baseline_images/test_magic_plot/hist_two.png b/src/tests/baseline_images/test_magic_plot/hist_two.png
diff --git a/src/tests/integration/baseline_images/test_questDB/custom_engine_histogram.png b/src/tests/integration/baseline_images/test_questDB/custom_engine_histogram.png
diff --git a/src/tests/integration/baseline_images/test_questDB/facet_wrap_custom_fill.png b/src/tests/integration/baseline_images/test_questDB/facet_wrap_custom_fill.png
diff --git a/...s/integration/baseline_images/test_questDB/facet_wrap_custom_fill_and_color.png b/...s/integration/baseline_images/test_questDB/facet_wrap_custom_fill_and_color.png
diff --git a/...ntegration/baseline_images/test_questDB/facet_wrap_custom_stacked_histogram.png b/...ntegration/baseline_images/test_questDB/facet_wrap_custom_stacked_histogram.png
diff --git a/...ation/baseline_images/test_questDB/facet_wrap_custom_stacked_histogram_cmap.png b/...ation/baseline_images/test_questDB/facet_wrap_custom_stacked_histogram_cmap.png
diff --git a/src/tests/integration/baseline_images/test_questDB/facet_wrap_default.png b/src/tests/integration/baseline_images/test_questDB/facet_wrap_default.png
diff --git a/...tests/integration/baseline_images/test_questDB/facet_wrap_default_no_legend.png b/...tests/integration/baseline_images/test_questDB/facet_wrap_default_no_legend.png
diff --git a/...sts/integration/baseline_images/test_questDB/histogram_categorical_combined.png b/...sts/integration/baseline_images/test_questDB/histogram_categorical_combined.png
diff --git a/...gration/baseline_images/test_questDB/histogram_numeric_categorical_combined.png b/...gration/baseline_images/test_questDB/histogram_numeric_categorical_combined.png
diff --git a/...line_images/test_questDB/histogram_numeric_categorical_combined_custom_fill.png b/...line_images/test_questDB/histogram_numeric_categorical_combined_custom_fill.png
diff --git a/...ages/test_questDB/histogram_numeric_categorical_combined_custom_multi_color.png b/...ages/test_questDB/histogram_numeric_categorical_combined_custom_multi_color.png
diff --git a/...mages/test_questDB/histogram_numeric_categorical_combined_custom_multi_fill.png b/...mages/test_questDB/histogram_numeric_categorical_combined_custom_multi_fill.png
diff --git a/...ests/integration/baseline_images/test_questDB/histogram_stacked_custom_cmap.png b/...ests/integration/baseline_images/test_questDB/histogram_stacked_custom_cmap.png
diff --git a/...sts/integration/baseline_images/test_questDB/histogram_stacked_custom_color.png b/...sts/integration/baseline_images/test_questDB/histogram_stacked_custom_color.png
diff --git a/...ration/baseline_images/test_questDB/histogram_stacked_custom_color_and_fill.png b/...ration/baseline_images/test_questDB/histogram_stacked_custom_color_and_fill.png
diff --git a/src/tests/integration/baseline_images/test_questDB/histogram_stacked_default.png b/src/tests/integration/baseline_images/test_questDB/histogram_stacked_default.png
diff --git a/...tests/integration/baseline_images/test_questDB/histogram_stacked_large_bins.png b/...tests/integration/baseline_images/test_questDB/histogram_stacked_large_bins.png