From 1b223f918ffb91da8f2d3cae98d632261943ca16 Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Wed, 23 Oct 2024 16:21:54 +0200 Subject: [PATCH 1/4] Deprecate .as_dict() on the builtin version of Row. The Spark-based Row does not support this, so clients should use .asDict() instead. --- src/databricks/labs/lsql/core.py | 25 ++++++++++++++++++------- tests/integration/test_core.py | 9 ++++++++- tests/unit/test_core.py | 2 +- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/databricks/labs/lsql/core.py b/src/databricks/labs/lsql/core.py index 856315b1..283a6e08 100644 --- a/src/databricks/labs/lsql/core.py +++ b/src/databricks/labs/lsql/core.py @@ -3,9 +3,11 @@ import json import logging import random +import sys import threading import time import types +import warnings from collections.abc import Callable, Iterator from datetime import timedelta from typing import TYPE_CHECKING, Any @@ -74,14 +76,23 @@ def factory(cls, col_names: list[str]) -> type: """Create a new Row class with the given column names.""" return type("Row", (Row,), {"__columns__": col_names}) - def as_dict(self) -> dict[str, Any]: - """Convert the row to a dictionary with the same conventions as Databricks SDK.""" - return dict(zip(self.__columns__, self, strict=True)) + if sys.version_info >= (3, 13): + + @warnings.deprecated("Using as_dict() on rows is deprecated; use asDict() instead.") # pylint: disable=no-member + def as_dict(self) -> dict[str, Any]: + """Convert the row to a dictionary with the same conventions as Databricks SDK.""" + return self.asDict() + else: + + def as_dict(self) -> dict[str, Any]: + """Convert the row to a dictionary with the same conventions as Databricks SDK.""" + warnings.warn("Using as_dict() on rows is deprecated; use asDict() instead.", DeprecationWarning) + return self.asDict() # PySpark's compatibility def asDict(self, recursive: bool = False) -> dict[str, Any]: _ = recursive - return self.as_dict() + return dict(zip(self.__columns__, self, strict=True)) def __eq__(self, other): """Check if the rows are equal.""" @@ -89,7 +100,7 @@ def __eq__(self, other): return False # compare rows as dictionaries, because the order # of fields in constructor is not guaranteed - return self.as_dict() == other.as_dict() + return self.asDict() == other.asDict() def __contains__(self, item): """Check if the column is in the row.""" @@ -311,7 +322,7 @@ def fetch_all( >>> pickup_time, dropoff_time = row[0], row[1] >>> pickup_zip = row.pickup_zip >>> dropoff_zip = row["dropoff_zip"] - >>> all_fields = row.as_dict() + >>> all_fields = row.asDict() >>> logger.info(f"{pickup_zip}@{pickup_time} -> {dropoff_zip}@{dropoff_time}: {all_fields}") :param statement: str @@ -366,7 +377,7 @@ def fetch_one(self, statement: str, disable_magic: bool = False, **kwargs) -> Ro >>> pickup_time, dropoff_time = row[0], row[1] >>> pickup_zip = row.pickup_zip >>> dropoff_zip = row['dropoff_zip'] - >>> all_fields = row.as_dict() + >>> all_fields = row.asDict() >>> print(f'{pickup_zip}@{pickup_time} -> {dropoff_zip}@{dropoff_time}: {all_fields}') :param statement: str diff --git a/tests/integration/test_core.py b/tests/integration/test_core.py index 375a03b7..21c505e1 100644 --- a/tests/integration/test_core.py +++ b/tests/integration/test_core.py @@ -45,7 +45,7 @@ def test_sql_execution_partial(ws, env_or_skip): pickup_time, dropoff_time = row[0], row[1] pickup_zip = row.pickup_zip dropoff_zip = row["dropoff_zip"] - all_fields = row.as_dict() + all_fields = row.asDict() logger.info(f"{pickup_zip}@{pickup_time} -> {dropoff_zip}@{dropoff_time}: {all_fields}") results.append((pickup_zip, dropoff_zip)) assert results == [ @@ -83,3 +83,10 @@ def test_fetch_value(ws): see = StatementExecutionExt(ws) count = see.fetch_value("SELECT COUNT(*) FROM samples.nyctaxi.trips") assert count == 21932 + + +def test_row_as_dict_deprecated(ws) -> None: + see = StatementExecutionExt(ws) + row = see.fetch_one("SELECT 1") + with pytest.deprecated_call(): + _ = row.as_dict() diff --git a/tests/unit/test_core.py b/tests/unit/test_core.py index 18e93549..00509c93 100644 --- a/tests/unit/test_core.py +++ b/tests/unit/test_core.py @@ -38,7 +38,7 @@ def test_row_from_kwargs(row): assert "foo" in row assert len(row) == 2 assert list(row) == ["bar", True] - assert row.as_dict() == {"foo": "bar", "enabled": True} + assert row.asDict() == {"foo": "bar", "enabled": True} foo, enabled = row assert foo == "bar" assert enabled is True From 406b7b62a85c3f24b0d37562d2f6a05f6745cfc1 Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Wed, 23 Oct 2024 16:23:08 +0200 Subject: [PATCH 2/4] Add some type hints. --- src/databricks/labs/lsql/core.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/databricks/labs/lsql/core.py b/src/databricks/labs/lsql/core.py index 283a6e08..abfab070 100644 --- a/src/databricks/labs/lsql/core.py +++ b/src/databricks/labs/lsql/core.py @@ -72,7 +72,7 @@ def __new__(cls, *args, **kwargs): return tuple.__new__(cls, args) @classmethod - def factory(cls, col_names: list[str]) -> type: + def factory(cls, col_names: list[str]) -> type["Row"]: """Create a new Row class with the given column names.""" return type("Row", (Row,), {"__columns__": col_names}) @@ -94,7 +94,7 @@ def asDict(self, recursive: bool = False) -> dict[str, Any]: _ = recursive return dict(zip(self.__columns__, self, strict=True)) - def __eq__(self, other): + def __eq__(self, other) -> bool: """Check if the rows are equal.""" if not isinstance(other, Row): return False @@ -102,7 +102,7 @@ def __eq__(self, other): # of fields in constructor is not guaranteed return self.asDict() == other.asDict() - def __contains__(self, item): + def __contains__(self, item) -> bool: """Check if the column is in the row.""" return item in self.__columns__ @@ -125,7 +125,7 @@ def __getattr__(self, col): except ValueError: raise AttributeError(col) from None - def __repr__(self): + def __repr__(self) -> str: """Get the string representation of the row.""" return f"Row({', '.join(f'{k}={v!r}' for (k, v) in zip(self.__columns__, self, strict=True))})" From 9777a0c3a54cbda2c0fb9fbbc680856226daf682 Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Wed, 23 Oct 2024 19:13:17 +0200 Subject: [PATCH 3/4] Fix linting issue. --- tests/integration/test_core.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/test_core.py b/tests/integration/test_core.py index 21c505e1..4983a0d0 100644 --- a/tests/integration/test_core.py +++ b/tests/integration/test_core.py @@ -88,5 +88,6 @@ def test_fetch_value(ws): def test_row_as_dict_deprecated(ws) -> None: see = StatementExecutionExt(ws) row = see.fetch_one("SELECT 1") + assert row is not None with pytest.deprecated_call(): _ = row.as_dict() From f53a36d7f91cd853b95f8c5b0810632702c0e613 Mon Sep 17 00:00:00 2001 From: Andrew Snare Date: Thu, 24 Oct 2024 14:44:59 +0200 Subject: [PATCH 4/4] Comment in why we have 2 versions of the method. --- src/databricks/labs/lsql/core.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/databricks/labs/lsql/core.py b/src/databricks/labs/lsql/core.py index abfab070..faec434c 100644 --- a/src/databricks/labs/lsql/core.py +++ b/src/databricks/labs/lsql/core.py @@ -76,6 +76,8 @@ def factory(cls, col_names: list[str]) -> type["Row"]: """Create a new Row class with the given column names.""" return type("Row", (Row,), {"__columns__": col_names}) + # If we can mark the method as deprecated via PEP 702 annotation, prefer this because it helps mypy and + # PyCharm/IntelliJ detect and flag deprecated use. if sys.version_info >= (3, 13): @warnings.deprecated("Using as_dict() on rows is deprecated; use asDict() instead.") # pylint: disable=no-member