Skip to content

Commit 97b63b1

Browse files
committed
Code quality improvements: dead code cleanup and test coverage
- Added comprehensive dead code analysis documentation in misc/docs/code-quality-improvements.md - Removed dead code: unreachable statements after raise, unused imports (getsource) - Added TODO comments to incomplete implementations (skip_lines, extension_based, delegate_as) - Added clarifying comments to signature template functions - Improved test coverage with new test files: - dol/tests/test_errors.py (15 tests) - coverage: 46% → 92% - dol/tests/test_dig.py (16 tests) - coverage: 46% → 93% - Overall test coverage improved from 58% to 59% - All tests passing (105 passed, 2 skipped)
1 parent de48027 commit 97b63b1

File tree

7 files changed

+679
-19
lines changed

7 files changed

+679
-19
lines changed

dol/base.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,6 +1014,7 @@ def rewind(self, instance):
10141014
instance.seek(0)
10151015

10161016
def skip_lines(self, instance, n_lines_to_skip=0):
1017+
# TODO: Implement line skipping - n_lines_to_skip parameter is currently unused
10171018
instance.seek(0)
10181019

10191020

dol/kv_codecs.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
def _string(string: str): ...
3131

3232

33+
# Signature template functions - not meant to be called, only used for signature composition
34+
# via @Sig decorator. Parameters appear "unused" but are actually captured for later use.
3335
@Sig
3436
def _csv_rw_sig(
3537
dialect: str = "excel",
@@ -591,3 +593,4 @@ def extension_based(
591593
):
592594
"""A factory that creates a key-value codec that uses the file extension to
593595
determine the value codec to use."""
596+
# TODO: Implement this function - ext_mapping parameter is currently unused

dol/tests/test_dig.py

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
1+
"""Tests for dol.dig module (layer introspection utilities)"""
2+
3+
import pytest
4+
from dol.dig import (
5+
get_first_attr_found,
6+
recursive_get_attr,
7+
re_get_attr,
8+
dig_up,
9+
store_trans_path,
10+
print_trans_path,
11+
last_element,
12+
inner_most,
13+
unravel_key,
14+
inner_most_key,
15+
next_layer,
16+
recursive_calls,
17+
layers,
18+
trace_getitem,
19+
not_found,
20+
no_default,
21+
)
22+
from io import StringIO
23+
import sys
24+
25+
26+
class SimpleStore:
27+
"""A simple test store with nested structure"""
28+
29+
def __init__(self, data, inner_store=None):
30+
self.data = data
31+
self.attr1 = "value1"
32+
if inner_store:
33+
self.store = inner_store
34+
35+
def _id_of_key(self, key):
36+
return f"id_{key}"
37+
38+
def _data_of_obj(self, obj):
39+
return f"data_{obj}"
40+
41+
42+
def test_get_first_attr_found():
43+
"""Test getting first found attribute"""
44+
store = SimpleStore({})
45+
store.attr2 = "value2"
46+
47+
# Should find first existing attribute
48+
result = get_first_attr_found(store, ["nonexistent", "attr1", "attr2"])
49+
assert result == "value1"
50+
51+
# Should find second if first doesn't exist
52+
result = get_first_attr_found(store, ["nonexistent", "attr2"])
53+
assert result == "value2"
54+
55+
56+
def test_get_first_attr_found_with_default():
57+
"""Test get_first_attr_found with default value"""
58+
store = SimpleStore({})
59+
60+
# Should return default when no attributes found
61+
result = get_first_attr_found(store, ["x", "y", "z"], default="default_value")
62+
assert result == "default_value"
63+
64+
65+
def test_get_first_attr_found_no_default():
66+
"""Test get_first_attr_found raises when no default and no attr found"""
67+
store = SimpleStore({})
68+
69+
with pytest.raises(AttributeError, match="None of the attributes were found"):
70+
get_first_attr_found(store, ["x", "y", "z"])
71+
72+
73+
def test_recursive_get_attr():
74+
"""Test recursive attribute lookup"""
75+
inner_store = SimpleStore({})
76+
inner_store.deep_attr = "deep_value"
77+
78+
outer_store = SimpleStore({}, inner_store=inner_store)
79+
80+
# Should find attribute in current store
81+
result = recursive_get_attr(outer_store, "attr1")
82+
assert result == "value1"
83+
84+
# Should recursively find in inner store
85+
result = recursive_get_attr(outer_store, "deep_attr")
86+
assert result == "deep_value"
87+
88+
# Should return default if not found
89+
result = recursive_get_attr(outer_store, "nonexistent", default="my_default")
90+
assert result == "my_default"
91+
92+
93+
def test_re_get_attr_and_dig_up_aliases():
94+
"""Test that re_get_attr and dig_up are aliases for recursive_get_attr"""
95+
assert re_get_attr is recursive_get_attr
96+
assert dig_up is recursive_get_attr
97+
98+
99+
def test_store_trans_path():
100+
"""Test store transformation path"""
101+
inner_store = SimpleStore({})
102+
outer_store = SimpleStore({}, inner_store=inner_store)
103+
104+
result = list(store_trans_path(outer_store, "key", "_id_of_key"))
105+
# Should yield transformed keys at each level
106+
assert "id_key" in result
107+
108+
109+
def test_print_trans_path(capsys):
110+
"""Test printing transformation path"""
111+
store = SimpleStore({})
112+
113+
# Capture stdout
114+
print_trans_path(store, "test", "_id_of_key")
115+
captured = capsys.readouterr()
116+
assert "test" in captured.out
117+
assert "id_test" in captured.out
118+
119+
120+
def test_print_trans_path_with_type(capsys):
121+
"""Test printing transformation path with type info"""
122+
store = SimpleStore({})
123+
124+
print_trans_path(store, "test", "_id_of_key", with_type=True)
125+
captured = capsys.readouterr()
126+
assert "<class 'str'>" in captured.out
127+
128+
129+
def test_last_element():
130+
"""Test getting last element from generator"""
131+
gen = (x for x in [1, 2, 3, 4, 5])
132+
assert last_element(gen) == 5
133+
134+
# Empty generator should return None
135+
gen = (x for x in [])
136+
assert last_element(gen) is None
137+
138+
139+
def test_inner_most():
140+
"""Test getting innermost transformation"""
141+
store = SimpleStore({})
142+
result = inner_most(store, "test", "_id_of_key")
143+
# Should return the final transformed value
144+
assert result is not None
145+
146+
147+
def test_next_layer():
148+
"""Test getting next layer of store"""
149+
inner_store = SimpleStore({})
150+
outer_store = SimpleStore({}, inner_store=inner_store)
151+
152+
# Should return inner store
153+
result = next_layer(outer_store)
154+
assert result is inner_store
155+
156+
# Should return not_found if no next layer
157+
result = next_layer(inner_store)
158+
assert result is not_found
159+
160+
161+
def test_recursive_calls():
162+
"""Test recursive function calls generator"""
163+
# Test with simple increment until sentinel
164+
def increment(x):
165+
if x >= 5:
166+
return not_found
167+
return x + 1
168+
169+
result = list(recursive_calls(increment, 0))
170+
assert result == [0, 1, 2, 3, 4, 5]
171+
172+
173+
def test_layers():
174+
"""Test getting all layers of a store"""
175+
inner_store = SimpleStore({})
176+
middle_store = SimpleStore({}, inner_store=inner_store)
177+
outer_store = SimpleStore({}, inner_store=middle_store)
178+
179+
result = layers(outer_store)
180+
assert len(result) == 3
181+
assert result[0] is outer_store
182+
assert result[1] is middle_store
183+
assert result[2] is inner_store
184+
185+
186+
def test_trace_getitem():
187+
"""Test tracing getitem operations through layers"""
188+
from dol.trans import wrap_kvs
189+
190+
# Create a simple layered store as shown in docstring
191+
d = {"a.num": "1000", "b.num": "2000"}
192+
193+
s = wrap_kvs(
194+
d,
195+
key_of_id=lambda x: x[: -len(".num")],
196+
id_of_key=lambda x: x + ".num",
197+
obj_of_data=lambda x: int(x),
198+
data_of_obj=lambda x: str(x),
199+
)
200+
201+
ss = wrap_kvs(
202+
s,
203+
key_of_id=lambda x: x.upper(),
204+
id_of_key=lambda x: x.lower(),
205+
)
206+
207+
# Trace should show transformation through layers
208+
trace = list(trace_getitem(ss, "A"))
209+
assert len(trace) > 0
210+
211+
# Check that trace includes _id_of_key and __getitem__ steps
212+
methods = [method for _, method, _ in trace]
213+
assert "_id_of_key" in methods
214+
assert "__getitem__" in methods
215+
216+
217+
def test_unravel_key():
218+
"""Test key unraveling (specialized store_trans_path)"""
219+
inner_store = SimpleStore({})
220+
outer_store = SimpleStore({}, inner_store=inner_store)
221+
222+
result = list(unravel_key(outer_store, "mykey"))
223+
# Should show key transformations
224+
assert len(result) > 0
225+
226+
227+
def test_inner_most_key():
228+
"""Test getting innermost key transformation"""
229+
store = SimpleStore({})
230+
231+
result = inner_most_key(store, "test")
232+
# Should return final key transformation or None
233+
assert result is None or isinstance(result, str)

0 commit comments

Comments
 (0)