Skip to content

Commit e83bcd2

Browse files
committed
fix: drop TableLayout.to_dict rather than add strict-mypy debt
I added to_dict() for surface consistency and then checked the gates properly: it added 8 errors under this package s strict mypy settings (dict[str, Any] plus comprehensions over the pydantic key models), taking the file from the 158 on main to 166. The alternative -- hand-building the dict from named fields -- is Any-free but silently drops any field a later spec adds to TablePartitionKey or TableSortKey, which is exactly the silent-drop failure this whole feature exists to prevent. So taking the reviewer s first option: no to_dict, with a comment recording why the inconsistency is deliberate, so the next reader does not "fix" it. A caller wanting dicts can map k.to_dict() itself. Also collapses a nested `with` flagged by ruff. Back to main s baseline exactly: mypy 158, ruff only the pre-existing long line in test_request_timeout.py. 139 passed.
1 parent 1312bd4 commit e83bcd2

2 files changed

Lines changed: 13 additions & 22 deletions

File tree

‎hotdata_framework/databases.py‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -57,19 +57,15 @@ class TableLayout:
5757
partition_by: list[TablePartitionKey]
5858
sorted_by: list[TableSortKey]
5959

60-
def to_dict(self) -> dict[str, Any]:
61-
"""Plain dict, like the other dataclasses here.
62-
63-
Not `asdict()`: the key lists hold pydantic models, which `asdict` copies
64-
through untouched, so the result would not be a plain dict. Each key is
65-
mapped through its own `to_dict()` instead.
66-
"""
67-
return {
68-
"schema_name": self.schema_name,
69-
"table_name": self.table_name,
70-
"partition_by": [k.to_dict() for k in self.partition_by],
71-
"sorted_by": [k.to_dict() for k in self.sorted_by],
72-
}
60+
# NO to_dict(), unlike every other dataclass here, and deliberately so.
61+
# `asdict()` would copy the pydantic key models through untouched rather than
62+
# flatten them, so it would not return a plain dict. Mapping each key through
63+
# its own `to_dict()` does flatten, but returns `dict[str, Any]` and adds
64+
# eight errors under this package's strict mypy settings; hand-building the
65+
# dict from named fields avoids that but silently drops any field a later
66+
# spec adds to the key models, which is the failure this whole feature exists
67+
# to prevent. A caller wanting dicts can map `k.to_dict()` itself and own
68+
# that choice.
7369

7470
@property
7571
def is_partitioned(self) -> bool:

‎tests/test_databases.py‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -515,10 +515,6 @@ def information_schema(self, **kwargs):
515515
assert captured["var_schema"] == "public"
516516
assert captured["table"] == "files"
517517
assert captured["connection_id"] == _MANAGED_DB.default_connection_id
518-
# to_dict flattens the pydantic keys rather than copying them through.
519-
assert layout.to_dict()["partition_by"] == [
520-
{"column": "event_date", "transform": "identity"}
521-
]
522518

523519

524520
def test_managed_table_layout_distinguishes_absent_from_unpartitioned():
@@ -555,9 +551,8 @@ def test_create_managed_database_refuses_layout_for_a_table_it_is_not_creating()
555551
parts, _ = _layout()
556552
client = _client()
557553

558-
with patch.object(client, "_databases_api") as dbs:
559-
with pytest.raises(ValueError, match="fils"):
560-
client.create_managed_database(
561-
"demo", tables=["files"], partition_by={"fils": parts}
562-
)
554+
with patch.object(client, "_databases_api") as dbs, pytest.raises(ValueError, match="fils"):
555+
client.create_managed_database(
556+
"demo", tables=["files"], partition_by={"fils": parts}
557+
)
563558
dbs.return_value.create_database.assert_not_called()

0 commit comments

Comments
 (0)