Uh oh!
There was an error while loading. Please reload this page.
FEAT: Add to_dict(), keys(), values(), items(), __contains__ to Row - #613
FEAT: Add to_dict(), keys(), values(), items(), __contains__ to Row #613Jahnvi Thakkar (jahnvi480) wants to merge 19 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds dict-like helpers (keys(), values(), items(), to_dict()) and __contains__ to the Row class so rows behave more like mappings, addressing issue #606. New unit tests cover each helper and the case-insensitive membership path.
Changes:
- Add
keys(),values(),items(),to_dict()toRow, sourced fromself._column_mapandself._values. - Add
__contains__with case-insensitive fallback viaself._column_map_lower. - Add four new tests in
tests/test_001_globals.pyexercising the helpers andinoperator (case-sensitive and case-insensitive).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| mssql_python/row.py | Implements the new mapping-style methods and membership protocol on Row. |
| tests/test_001_globals.py | Adds tests for to_dict, keys/values/items, and __contains__ (case-sensitive + insensitive). |
Key concern: in real cursor-built rows, _column_map stores each column under both its original-cased name and a lowercase alias, so keys()/items()/to_dict() will emit each column twice and len(keys()) != len(values()). The new tests use a hand-built column_map without the lowercase aliases, so they don't catch this — see the inline comment for details and suggested directions.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 59.9%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 76.6%
mssql_python.__init__.py: 77.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 81.4%
mssql_python.row.py: 82.5%
mssql_python.pybind.connection.connection.cpp: 84.3%
mssql_python.logging.py: 85.5%🔗 Quick Links
|
…reusability, add integration tests
Gaurav Sharma (bewithgaurav)
left a comment
There was a problem hiding this comment.
perf regression on Row.__init__ from the column_names kwarg - fix suggestion + details inline
two other suggestions on type annotations, docstring)
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| raise AttributeError(f"Row has no attribute '{name}'") | ||
| def _get_column_names(self) -> tuple: |
There was a problem hiding this comment.
regression caused from previous suggestion:
_get_column_names() reads self._cursor.description lazily on first call. if the cursor re-executes before keys()/to_dict() is called on an old row, the old row picks up the new query's column names. repro:
cursor.execute("SELECT 1 AS OrderID, 'Widget' AS Product, 9.99 AS Price")
order=cursor.fetchone()
cursor.execute("SELECT 100 AS CustomerID, 'Alice' AS Name")
customer=cursor.fetchone()
order.to_dict()
# expected: {'OrderID': 1, 'Product': 'Widget', 'Price': 9.99}# actual: {'CustomerID': 1, 'Name': 'Widget'}# Price silently dropped (zip truncates 3 values to 2 columns)fix: snapshot cursor.description at construction time instead of reading it live. one pointer assignment, no computation:
def__init__(self, ...):
self._column_names=Noneself._description=cursor.descriptionifcursorelseNonedef_get_column_names(self) ->tuple:
ifself._column_namesisnotNone:
returnself._column_namesifself._description:
column_names=tuple(desc[0] fordescinself._description)
elifself._column_map:
...still lazy, still zero-cost if keys()/to_dict() is never called, but safe on cursor reuse.
| self._column_names = column_names | ||
| return column_names | ||
| def keys(self) -> tuple: |
There was a problem hiding this comment.
just checked during testing, adding keys(), values(), items(), to_dict() shadows column attribute access for any column with those names. repro:
cursor.execute("SELECT 'timeout' AS keys, '30' AS [values]")
row=cursor.fetchone()
row.keys# before PR: 'timeout' (via __getattr__)row.keys# after PR: <bound method Row.keys>row["keys"] # still works: 'timeout'class methods win over __getattr__ in Python's MRO. this is a silent breaking change for existing code using row.keys or row.values as column attribute access.
| """Return (column_name, value) pairs, like dict.items().""" | ||
| return list(zip(self._get_column_names(), self._values)) | ||
| def to_dict(self) -> dict: |
There was a problem hiding this comment.
to_dict() silently loses data when the query has duplicate column names. common in joins:
cursor.execute(""" SELECT e.id, e.name, m.id, m.name FROM employees e LEFT JOIN employees m ON e.manager_id = m.id""")
row=cursor.fetchone()
list(row) # [1, 'Alice', 2, 'Bob'] — 4 valuesrow.to_dict() # {'id': 2, 'name': 'Bob'} — 2 entries, Alice gonelets discuss approaching this
Work Item / Issue Reference
Summary
This pull request adds new dictionary-like methods and membership testing to the
Rowclass inmssql_python/row.py, and introduces corresponding unit tests to ensure their correct behavior. These enhancements makeRowinstances more intuitive and compatible with Python's mapping protocol, improving usability and testability.Enhancements to the
Rowclass:keys(),values(), anditems()methods toRow, allowing access to column names, values, and (name, value) pairs in a dictionary-like fashion.to_dict()method to return the row as a plain dictionary mapping column names to values.inoperator to test for column membership, including case-insensitive matching when a lowercase column map is provided.Testing improvements:
to_dict(),keys(),values(), anditems()methods to verify they behave like theirdictcounterparts.inoperator, including both case-sensitive and case-insensitive membership checks.