Repository: ploomber/jupysql
Pull request #89 (2023-01-25 21:34:15)

82 - More Telemetry
Resolve https://github.com/ploomber/jupysql/issues/82

Add more telemetry 

boxplot
<img width="1415" alt="Screenshot 2023-01-25 at 4 57 07 PM" src="https://user-images.githubusercontent.com/123580782/214701371-f05faf83-3a12-4e7e-98e7-b9f30f6d57a2.png">



historgram
<img width="1416" alt="Screenshot 2023-01-25 at 5 00 42 PM" src="https://user-images.githubusercontent.com/123580782/214701354-cab0314e-ec80-4159-95b8-a994cc96cc78.png">



<!-- readthedocs-preview jupysql start -->
----
:books: Documentation preview :books:: https://jupysql--89.org.readthedocs.build/en/89/

<!-- readthedocs-preview jupysql end -->

idomic: @tonykploomber We can probably add it to the `%sqlrender` , and saving/loading the query and the `SQL Store`.
@edublancas anything else that we're missing here?

It'd be also interesting to see the distribution of DB/DWH types used, but maybe that's v2.
tonykploomber: @idomic Please review again, I think we can add more test cases cover other telemetry actions (`sqlrender, init, execute, data-frame...`) in another PR

Snapshot provided as local pytest result: PASS

<img width="1732" alt="Screenshot 2023-01-26 at 11 07 35 PM" src="https://user-images.githubusercontent.com/123580782/215007653-97c099b3-04d7-430b-9d4c-6b113850521d.png">

idomic: @tonykploomber awesome!
Let's cover all of the missing parts in this PR so we can merge and release.
idomic: @tonykploomber I see other commits as part of this pr, are you sure you rebased?
tonykploomber: For the CHANGELOG, the latest released version is `0.5.2`, I guess I can add to `0.5.3`
tonykploomber: Somehow Read the Docs build did not success...

Patch:
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index 7c3acda..8dc9734 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -7,7 +7,7 @@ jobs:
     strategy:
       matrix:
         python-version: [3.7, 3.8, 3.9, '3.10', '3.11']
-        os: [ubuntu-latest, macos-latest]
+        os: [ubuntu-latest, macos-latest, windows-latest]
 
     runs-on: ${{ matrix.os }}
 
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b135261..037bfb2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
 # CHANGELOG
 
 ## 0.5.3dev
+* [Fix] Adds telemetry collect for `%sqlplot` invocation ([#89](https://github.com/ploomber/jupysql/pull/89))
 * [Fix] `setup.py` fix due to change in setuptools 67.0.0
 
 ## 0.5.2 (2023-01-03)
diff --git a/src/sql/plot.py b/src/sql/plot.py
index 718d9a3..ffe5c52 100644
--- a/src/sql/plot.py
+++ b/src/sql/plot.py
@@ -17,6 +17,7 @@ except ModuleNotFoundError:
 
 from sql.store import store
 import sql.connection
+from sql.telemetry import telemetry
 
 
 def _summary_stats(con, table, column, with_=None):
@@ -190,6 +191,7 @@ def _boxplot_stats(con, table, column, whis=1.5, autorange=False, with_=None):
 
 # https://github.com/matplotlib/matplotlib/blob/ddc260ce5a53958839c244c0ef0565160aeec174/lib/matplotlib/axes/_axes.py#L3915
 @requires(["matplotlib"])
+@telemetry.log_call("boxplot")
 def boxplot(table, column, *, orient="v", with_=None, conn=None):
     """Plot boxplot
 
@@ -280,6 +282,7 @@ FROM "{{table}}"
 
 
 @requires(["matplotlib"])
+@telemetry.log_call("histogram")
 def histogram(table, column, bins, with_=None, conn=None):
     """Plot histogram
 


Test patch:
diff --git a/src/tests/test_magic.py b/src/tests/test_magic.py
index 92904bc..81e2cd0 100644
--- a/src/tests/test_magic.py
+++ b/src/tests/test_magic.py
@@ -1,3 +1,4 @@
+import platform
 from pathlib import Path
 import os.path
 import re
@@ -154,18 +155,21 @@ def test_connection_args_enforce_json(ip):
     assert result.error_in_exec
 
 
+@pytest.mark.skipif(platform.system() == "Windows", reason="failing on windows")
 def test_connection_args_in_connection(ip):
     ip.run_cell('%sql --connection_arguments {"timeout":10} sqlite:///:memory:')
     result = ip.run_cell("%sql --connections")
     assert "timeout" in result.result["sqlite:///:memory:"].connect_args
 
 
+@pytest.mark.skipif(platform.system() == "Windows", reason="failing on windows")
 def test_connection_args_single_quotes(ip):
     ip.run_cell("%sql --connection_arguments '{\"timeout\": 10}' sqlite:///:memory:")
     result = ip.run_cell("%sql --connections")
     assert "timeout" in result.result["sqlite:///:memory:"].connect_args
 
 
+@pytest.mark.skipif(platform.system() == "Windows", reason="failing on windows")
 def test_connection_args_double_quotes(ip):
     ip.run_cell('%sql --connection_arguments "{\\"timeout\\": 10}" sqlite:///:memory:')
     result = ip.run_cell("%sql --connections")
diff --git a/src/tests/test_telemetry.py b/src/tests/test_telemetry.py
new file mode 100644
index 0000000..910215c
--- /dev/null
+++ b/src/tests/test_telemetry.py
@@ -0,0 +1,87 @@
+from pathlib import Path
+from unittest.mock import ANY, Mock
+import pytest
+import urllib.request
+import duckdb
+from sql.telemetry import telemetry
+from sql import plot
+
+# Ref: https://pytest.org/en/7.2.x/how-to/tmp_path.html#
+# Utilize tmp directory to store downloaded csv
+
+
+@pytest.fixture
+def simple_file_path(tmpdir):
+    file_path_str = str(tmpdir.join("iris.csv"))
+
+    if not Path(file_path_str).is_file():
+        urllib.request.urlretrieve(
+            "https://raw.githubusercontent.com/plotly/datasets/master/iris-data.csv",
+            file_path_str,
+        )
+
+    yield file_path_str
+
+
+@pytest.fixture
+def simple_db_conn():
+    conn = duckdb.connect(database=":memory:")
+    return conn
+
+
+@pytest.fixture
+def mock_log_api(monkeypatch):
+    mock_log_api = Mock()
+    monkeypatch.setattr(telemetry, "log_api", mock_log_api)
+    yield mock_log_api
+
+
+def test_boxplot_telemetry_execution(mock_log_api, simple_db_conn, simple_file_path):
+    plot.boxplot(simple_file_path, "petal width", conn=simple_db_conn, orient="h")
+
+    mock_log_api.assert_called_with(
+        action="jupysql-boxplot-success", total_runtime=ANY, metadata=ANY
+    )
+
+
+def test_histogram_telemetry_execution(mock_log_api, simple_db_conn, simple_file_path):
+    # Test the injected log_api gets called
+    plot.histogram(simple_file_path, "petal width", bins=50, conn=simple_db_conn)
+
+    mock_log_api.assert_called_with(
+        action="jupysql-histogram-success", total_runtime=ANY, metadata=ANY
+    )
+
+
+def test_data_frame_telemetry_execution(mock_log_api, ip, simple_file_path):
+    # Simulate the cell query & get the DataFrame
+    ip.run_cell("%sql duckdb://")
+    ip.run_cell("result = %sql SELECT * FROM read_csv_auto('" + simple_file_path + "')")
+    ip.run_cell("result.DataFrame()")
+    mock_log_api.assert_called_with(
+        action="jupysql-data-frame-success", total_runtime=ANY, metadata=ANY
+    )
+
+
+def test_sqlrender_telemetry_execution(mock_log_api, ip, simple_file_path):
+    # Simulate the sqlrender query
+    ip.run_cell("%sql duckdb://")
+    ip.run_cell(
+        "%sql --save class_setosa --no-execute \
+            SELECT * FROM read_csv_auto('"
+        + simple_file_path
+        + "' WHERE class='Iris-setosa'"
+    )
+    ip.run_cell("%sqlrender class_setosa")
+
+    mock_log_api.assert_called_with(
+        action="jupysql-sqlrender-success", total_runtime=ANY, metadata=ANY
+    )
+
+
+def test_execute_telemetry_execution(mock_log_api, ip):
+    ip.run_cell("%sql duckdb://")
+
+    mock_log_api.assert_called_with(
+        action="jupysql-execute-success", total_runtime=ANY, metadata=ANY
+    )