Repository: ploomber/jupysql
Pull request #462 (2023-04-27 14:14:45)

Change default displaylimit value
## Describe your changes

- Change default display limit as -> `10`
- Add unlimited display limit support if user sets value as `0` or `None`

### Demo

Case 1: Unset -> Default display limit as  10 (Unset)

https://user-images.githubusercontent.com/123580782/235429111-4233f783-16f9-4cda-a8ec-9beb88ac274b.mov

Case 2: Set as 20

<img width="824" alt="Screenshot 2023-05-01 at 4 28 47 PM" src="https://user-images.githubusercontent.com/123580782/235429137-16c9b78b-3cc3-46ae-b5ca-64ee12520ca6.png">

Case 3: Set as None (no limit)

<img width="943" alt="Screenshot 2023-05-01 at 4 32 06 PM" src="https://user-images.githubusercontent.com/123580782/235429366-5852f0c9-61fc-4ea3-b709-c29d33027373.png">


[Doc preview](https://jupysql--462.org.readthedocs.build/en/462/api/configuration.html#displaylimit)

<img width="838" alt="Screenshot 2023-04-27 at 11 44 33 PM" src="https://user-images.githubusercontent.com/123580782/234915488-b34df3c0-06e9-4d59-9f67-263aa32ec1cf.png">



## Issue number

Closes https://github.com/ploomber/jupysql/issues/419

## Checklist before requesting a review

- [ ] Performed a self-review of my code
- [ ] Formatted my code with [`pkgmt format`](https://ploomber-contributing.readthedocs.io/en/latest/contributing/pr.html#linting-formatting)
- [ ] Added [tests](https://ploomber-contributing.readthedocs.io/en/latest/contributing/pr.html#testing) (when necessary).
- [ ] Added [docstring](https://ploomber-contributing.readthedocs.io/en/latest/contributing/pr.html#documenting-changes-and-new-features) documentation and update the [changelog](https://ploomber-contributing.readthedocs.io/en/latest/contributing/pr.html#changelog) (when needed)



<!-- readthedocs-preview jupysql start -->
----
:books: Documentation preview :books:: https://jupysql--462.org.readthedocs.build/en/462/

<!-- readthedocs-preview jupysql end -->

edublancas: please mark as draft if this isn't ready for review. also, did you see my comment? https://github.com/ploomber/jupysql/issues/419#issuecomment-1522456875
tonykploomber: Added some test cases to cover more

Now the default value is 10

When config value is:
1. `0`, it will render all table rows
2. `None`, it will still use default value (This is unexpected behavior since we think the value should be int now, but we still add the special handle for this)
3. Negative Integer, will set the config value unsuccessfully
4. Positive Integer, will set the config value successfully
tonykploomber: Link to [displaylimit](https://jupysql.ploomber.io/en/latest/api/configuration.html#displaylimit) was added 

<img width="716" alt="Screenshot 2023-05-05 at 10 23 00 PM" src="https://user-images.githubusercontent.com/123580782/236486149-0a4bb964-ff6d-4597-9a16-097d7ef416f2.png">

tonykploomber: @yafimvo 
1. Now `None` and `0` will be no limit setup now. Please test again

tonykploomber: > lgtm, @tonykploomber please fix the display limit from above, it's still on 60 for some reason.

Please check [the doc](https://jupysql--462.org.readthedocs.build/en/462/api/configuration.html#displaylimit) again!

From my side, it shows 10

<img width="811" alt="Screenshot 2023-05-09 at 10 48 49 AM" src="https://user-images.githubusercontent.com/123580782/236981450-7ded6c1d-b3dd-4c9a-8fb9-1293335fed1f.png">

yafimvo: > Please check [the doc](https://jupysql--462.org.readthedocs.build/en/462/api/configuration.html#displaylimit) again!

@tonykploomber I think the issue is with [this section](https://jupysql--462.org.readthedocs.build/en/462/api/configuration.html#options)

![image](https://user-images.githubusercontent.com/103026689/237044663-c27fd1bd-c5b8-437e-b5f5-db6c6cc0c311.png)

yafimvo: @tonykploomber is this ready?
tonykploomber: @yafimvo 
I think I tried to rebuild the doc and still see the preview shows old config value -> `60`

However, I re-raised [another PR](https://github.com/ploomber/jupysql/pull/487) with [preview](https://jupysql--487.org.readthedocs.build/en/487/api/configuration.html#options) and it shows new config value -> `10`


<img width="839" alt="Screenshot 2023-05-14 at 10 58 05 PM" src="https://github.com/ploomber/jupysql/assets/123580782/027af3de-af88-4101-a12d-ef4e6d868dfb">


I think it might be some cache issue in readthedocs, do you want to merge the [another PR ](https://github.com/ploomber/jupysql/pull/487) instead of this one? Although they are same code changes
yafimvo: Yes, I think it's a cache issue. I ran `%config SqlMagic` on Binder and it shows the correct number.

lgtm.

@idomic 
tonykploomber: @idomic 
This should be good to ship

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9d64068..b80ca56 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@
 
 * [Doc] documenting `%sqlcmd tables`/`%sqlcmd columns`
 * [Feature] Better error messages when function used in plotting API unsupported by DB driver (#159)
+* [Fix] Fix the default value of %config SqlMagic.displaylimit to 10 (#462)
 
 ## 0.7.4 (2023-04-28)
 No changes
diff --git a/doc/api/configuration.md b/doc/api/configuration.md
index 607cabb..8a93196 100644
--- a/doc/api/configuration.md
+++ b/doc/api/configuration.md
@@ -28,6 +28,8 @@ set (usually with a `LIMIT` clause in the SQL).  `displaylimit` is similar,
 but the entire result set is still pulled into memory (for later analysis);
 only the screen display is truncated.
 
+If you are concerned about query performance, please use the `autolimit` config.
+
 +++
 
 ## Setup
@@ -105,10 +107,12 @@ Automatically limit the size of the returned result sets (e.g., add a `LIMIT` at
 
 ## `displaylimit`
 
-Default: `None` (no limit)
+Default: `10`
 
 Automatically limit the number of rows displayed (full result set is still stored).
 
+(To display all rows: set to `0` or `None`)
+
 ```{code-cell} ipython3
 %config SqlMagic.displaylimit = None
 %sql SELECT * FROM languages
@@ -185,7 +189,9 @@ for _ in range(100):
 
 %sql SELECT * FROM points
 ```
+
 To unset:
+
 ```{code-cell} ipython3
 %config SqlMagic.polars_dataframe_kwargs = {}
 ```
@@ -216,4 +222,4 @@ INSERT INTO points VALUES (1, 1);
 CREATE TABLE more_points (x, y);
 INSERT INTO more_points VALUES (0, 0);
 INSERT INTO more_points VALUES (1, 1);
-```
\ No newline at end of file
+```
diff --git a/doc/community/vs.md b/doc/community/vs.md
index af55680..08bd6ae 100644
--- a/doc/community/vs.md
+++ b/doc/community/vs.md
@@ -15,4 +15,5 @@ If you're migrating from `ipython-sql` to JupySQL, these are the differences (in
 - JupySQL allows you to break queries into multiple cells with the help of CTEs. [Click here](../compose) to learn more.
 - Using `%sqlcmd tables` and `%sqlcmd columns --table/-t` user can quickly explore tables in the database and the columns each table has. [Click here](../user-guide/tables-columns) to learn more.
 - [Polars Integration](../integrations/polars) to convert query results to `polars.DataFrame`. `%config SqlMagic.autopolars` can be used to automatically return Polars DataFrames instead of regular result sets.
-- Integration tests with PostgreSQL, MariaDB, MySQL, SQLite and DuckDB.
\ No newline at end of file
+- Integration tests with PostgreSQL, MariaDB, MySQL, SQLite and DuckDB.
+- The configuration default value of SqlMagic.displaylimit is different, in JupySQL is `10`, whereas in ipython-sql is `None`
\ No newline at end of file
diff --git a/src/sql/magic.py b/src/sql/magic.py
index 9b3a5d5..3a07209 100644
--- a/src/sql/magic.py
+++ b/src/sql/magic.py
@@ -30,7 +30,7 @@ from sql._patch import patch_ipython_usage_error
 from ploomber_core.dependencies import check_installed
 
 from traitlets.config.configurable import Configurable
-from traitlets import Bool, Int, Unicode, Dict, observe
+from traitlets import Bool, Int, TraitError, Unicode, Dict, observe, validate
 
 try:
     from pandas.core.frame import DataFrame, Series
@@ -94,7 +94,7 @@ class SqlMagic(Magics, Configurable):
         help="Don't display the full traceback on SQL Programming Error",
     )
     displaylimit = Int(
-        None,
+        sql.run.DEFAULT_DISPLAYLIMIT_VALUE,
         config=True,
         allow_none=True,
         help=(
@@ -144,6 +144,26 @@ class SqlMagic(Magics, Configurable):
         # Add ourself to the list of module configurable via %config
         self.shell.configurables.append(self)
 
+    # To verify displaylimit is valid positive integer
+    # If:
+    #   None -> We treat it as 0 (no limit)
+    #   Positive Integer -> Pass
+    #   Negative Integer -> raise Error
+    @validate("displaylimit")
+    def _valid_displaylimit(self, proposal):
+        if proposal["value"] is None:
+            print("displaylimit: Value None will be treated as 0 (no limit)")
+            return 0
+        try:
+            value = int(proposal["value"])
+            if value < 0:
+                raise TraitError(
+                    "{}: displaylimit cannot be a negative integer".format(value)
+                )
+            return value
+        except ValueError:
+            raise TraitError("{}: displaylimit is not an integer".format(value))
+
     @observe("autopandas", "autopolars")
     def _mutex_autopandas_autopolars(self, change):
         # When enabling autopandas or autopolars, automatically disable the
diff --git a/src/sql/run.py b/src/sql/run.py
index 2197fed..88911d6 100644
--- a/src/sql/run.py
+++ b/src/sql/run.py
@@ -25,6 +25,8 @@ import logging
 import warnings
 from collections.abc import Iterable
 
+DEFAULT_DISPLAYLIMIT_VALUE = 10
+
 
 def unduplicate_field_names(field_names):
     """Append a number to duplicate field names to make them unique."""
@@ -154,12 +156,17 @@ class ResultSet(ColumnGuesserMixin):
             # to create clickable links
             result = html.unescape(result)
             result = _cell_with_spaces_pattern.sub(_nonbreaking_spaces, result)
-            if self.config.displaylimit and len(self) > self.config.displaylimit:
+            if len(self) > self.pretty.row_count:
                 HTML = (
                     '%s\n<span style="font-style:italic;text-align:center;">'
                     "%d rows, truncated to displaylimit of %d</span>"
+                    "<br>"
+                    '<span style="font-style:italic;text-align:center;">'
+                    "If you want to see more, please visit "
+                    '<a href="https://jupysql.ploomber.io/en/latest/api/configuration.html#displaylimit">displaylimit</a>'  # noqa: E501
+                    " configuration</span>"
                 )
-                result = HTML % (result, len(self), self.config.displaylimit)
+                result = HTML % (result, len(self), self.pretty.row_count)
             return result
         else:
             return None
@@ -562,7 +569,7 @@ def raw_run(conn, sql):
 class PrettyTable(prettytable.PrettyTable):
     def __init__(self, *args, **kwargs):
         self.row_count = 0
-        self.displaylimit = None
+        self.displaylimit = DEFAULT_DISPLAYLIMIT_VALUE
         return super(PrettyTable, self).__init__(*args, **kwargs)
 
     def add_rows(self, data):
@@ -571,7 +578,7 @@ class PrettyTable(prettytable.PrettyTable):
         self.clear_rows()
         self.displaylimit = data.config.displaylimit
         if self.displaylimit == 0:
-            self.displaylimit = None  # TODO: remove this to make 0 really 0
+            self.displaylimit = None
         if self.displaylimit in (None, 0):
             self.row_count = len(data)
         else:


Test patch:
diff --git a/src/tests/test_magic.py b/src/tests/test_magic.py
index a98360e..c1a21a0 100644
--- a/src/tests/test_magic.py
+++ b/src/tests/test_magic.py
@@ -1,3 +1,4 @@
+import logging
 import sqlite3
 import platform
 from pathlib import Path
@@ -231,15 +232,31 @@ def test_connection_args_double_quotes(ip):
 #     assert 'Shakespeare' in str(persisted)
 
 
-@pytest.mark.parametrize("value", ["None", "0"])
-def test_displaylimit_disabled(ip, value):
-    ip.run_line_magic("config", "SqlMagic.autolimit = None")
+def test_displaylimit_no_limit(ip):
+    ip.run_line_magic("config", "SqlMagic.displaylimit = 0")
 
-    ip.run_line_magic("config", f"SqlMagic.displaylimit = {value}")
-    result = runsql(ip, "SELECT * FROM author;")
+    out = ip.run_cell("%sql SELECT * FROM number_table;")
+    assert out.result == [
+        (4, -2),
+        (-5, 0),
+        (2, 4),
+        (0, 2),
+        (-5, -1),
+        (-2, -3),
+        (-2, -3),
+        (-4, 2),
+        (2, -5),
+        (4, 3),
+    ]
 
-    assert "Brecht" in result._repr_html_()
-    assert "Shakespeare" in result._repr_html_()
+
+def test_displaylimit_default(ip):
+    # Insert extra data to make number_table bigger (over 10 to see truncated string)
+    ip.run_cell("%sql INSERT INTO number_table VALUES (4, 3)")
+    ip.run_cell("%sql INSERT INTO number_table VALUES (4, 3)")
+
+    out = runsql(ip, "SELECT * FROM number_table;")
+    assert "truncated to displaylimit of 10" in out._repr_html_()
 
 
 def test_displaylimit(ip):
@@ -252,6 +269,88 @@ def test_displaylimit(ip):
     assert "Shakespeare" not in result._repr_html_()
 
 
+@pytest.mark.parametrize("config_value, expected_length", [(3, 3), (6, 6)])
+def test_displaylimit_enabled_truncated_length(ip, config_value, expected_length):
+    # Insert extra data to make number_table bigger (over 10 to see truncated string)
+    ip.run_cell("%sql INSERT INTO number_table VALUES (4, 3)")
+    ip.run_cell("%sql INSERT INTO number_table VALUES (4, 3)")
+
+    ip.run_cell(f"%config SqlMagic.displaylimit = {config_value}")
+    out = runsql(ip, "SELECT * FROM number_table;")
+    assert f"truncated to displaylimit of {expected_length}" in out._repr_html_()
+
+
+@pytest.mark.parametrize("config_value", [(None), (0)])
+def test_displaylimit_enabled_no_limit(
+    ip,
+    config_value,
+):
+    # Insert extra data to make number_table bigger (over 10 to see truncated string)
+    ip.run_cell("%sql INSERT INTO number_table VALUES (4, 3)")
+    ip.run_cell("%sql INSERT INTO number_table VALUES (4, 3)")
+
+    ip.run_cell(f"%config SqlMagic.displaylimit = {config_value}")
+    out = runsql(ip, "SELECT * FROM number_table;")
+    assert "truncated to displaylimit of " not in out._repr_html_()
+
+
+@pytest.mark.parametrize(
+    "config_value, expected_error_msg",
+    [
+        (-1, "displaylimit cannot be a negative integer"),
+        (-2, "displaylimit cannot be a negative integer"),
+        (-2.5, "The 'displaylimit' trait of a SqlMagic instance expected an int"),
+        (
+            "'some_string'",
+            "The 'displaylimit' trait of a SqlMagic instance expected an int",
+        ),
+    ],
+)
+def test_displaylimit_enabled_with_invalid_values(
+    ip, config_value, expected_error_msg, caplog
+):
+    with caplog.at_level(logging.ERROR):
+        ip.run_cell(f"%config SqlMagic.displaylimit = {config_value}")
+
+    assert expected_error_msg in caplog.text
+
+
+@pytest.mark.parametrize(
+    "query_clause, expected_truncated_length",
+    [
+        # With limit
+        ("SELECT * FROM number_table", 12),
+        ("SELECT * FROM number_table LIMIT 5", None),
+        ("SELECT * FROM number_table LIMIT 10", None),
+        ("SELECT * FROM number_table LIMIT 11", 11),
+        # With conditions
+        ("SELECT * FROM number_table WHERE x > 0", None),
+        ("SELECT * FROM number_table WHERE x < 0", None),
+        ("SELECT * FROM number_table WHERE y < 0", None),
+        ("SELECT * FROM number_table WHERE y > 0", None),
+    ],
+)
+@pytest.mark.parametrize("is_saved_by_cte", [(True, False)])
+def test_displaylimit_with_conditional_clause(
+    ip, query_clause, expected_truncated_length, is_saved_by_cte
+):
+    # Insert extra data to make number_table bigger (over 10 to see truncated string)
+    ip.run_cell("%sql INSERT INTO number_table VALUES (4, 3)")
+    ip.run_cell("%sql INSERT INTO number_table VALUES (4, 3)")
+
+    if is_saved_by_cte:
+        ip.run_cell(f"%sql --save saved_cte --no-execute {query_clause}")
+        out = ip.run_line_magic("sql", "--with saved_cte SELECT * from saved_cte")
+    else:
+        out = runsql(ip, query_clause)
+
+    if expected_truncated_length:
+        assert (
+            f"{expected_truncated_length} rows, truncated to displaylimit of 10"
+            in out._repr_html_()
+        )
+
+
 def test_column_local_vars(ip):
     ip.run_line_magic("config", "SqlMagic.column_local_vars = True")
     result = runsql(ip, "SELECT * FROM author;")