Skip to content

Commit 817c0b8

Browse files
rustyconoverclaude
rustyconover
and
claude
committed
chore: adopt vgi-python ruff/mypy/pydoclint code-quality config
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2f0ca15 commit 817c0b8

14 files changed

Lines changed: 184 additions & 48 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,18 @@ jobs:
6262
- name: Install dependencies
6363
run: uv sync --frozen --all-extras
6464

65+
- name: Check formatting
66+
run: uv run --no-sync ruff format --check .
67+
6568
- name: Check linting
6669
run: uv run --no-sync ruff check .
6770

6871
- name: Type check (mypy)
6972
run: uv run --no-sync mypy vgi_news/ news_worker.py
7073

74+
- name: Docstring lint (pydoclint)
75+
run: uv run --no-sync pydoclint vgi_news/ news_worker.py
76+
7177
# Resolve the latest published haybarn release once, so the whole matrix tests
7278
# the same version (and we never hardcode/pin it).
7379
resolve-haybarn:

‎conftest.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
1-
# Presence of this file puts the repo root on sys.path so tests can
2-
# `import news_worker` and `import vgi_news`.
1+
"""Root conftest: presence puts the repo root on ``sys.path``.
2+
3+
This lets tests ``import news_worker`` and ``import vgi_news``.
4+
"""

‎pyproject.toml‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ dev = [
1818
"pytest>=8",
1919
"ruff>=0.6",
2020
"mypy>=1.10",
21+
"pydoclint>=0.5",
2122
]
2223

2324
[build-system]
@@ -40,9 +41,33 @@ line-length = 120
4041
target-version = "py313"
4142

4243
[tool.ruff.lint]
43-
select = ["E", "F", "I", "UP", "B"]
44+
select = ["E", "F", "I", "UP", "B", "SIM", "D"]
45+
46+
[tool.ruff.lint.per-file-ignores]
47+
# Docstrings (D) are enforced on the worker package/script (product code), not tests.
48+
"tests/**" = ["D"]
49+
# The PEP 723 inline-script header wraps dependency pins that should stay one-per-line.
50+
"news_worker.py" = ["E501"]
51+
52+
[tool.ruff.lint.pydocstyle]
53+
convention = "google"
54+
55+
[tool.ruff.format]
56+
quote-style = "double"
57+
58+
[tool.pydoclint]
59+
# Docstring consistency gate (complements ruff's D rules: ruff checks docstring
60+
# shape, pydoclint checks documented args/returns/attrs match the code).
61+
style = "google"
62+
arg_type_hints_in_docstring = false
63+
check_return_types = false
64+
check_yield_types = false
65+
check_class_attributes = true
66+
skip_checking_raises = true
67+
allow_init_docstring = true
4468

4569
[tool.mypy]
4670
python_version = "3.13"
71+
strict = true
72+
warn_return_any = true
4773
warn_unused_ignores = true
48-
ignore_missing_imports = true

‎scripts/run_sql_e2e.py‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,35 @@
3838
# assert the window scan-state round-trips across a batch boundary.
3939
_GDELT_PAGE_1 = {
4040
"articles": [
41-
{"url": "https://a.test/1", "title": "Newest A", "seendate": "20240615T120000Z",
42-
"domain": "a.test", "language": "English", "sourcecountry": "United States", "socialimage": "x"},
43-
{"url": "https://a.test/2", "title": "Newest B", "seendate": "20240615T100000Z",
44-
"domain": "a.test", "language": "English", "sourcecountry": "United States"},
41+
{
42+
"url": "https://a.test/1",
43+
"title": "Newest A",
44+
"seendate": "20240615T120000Z",
45+
"domain": "a.test",
46+
"language": "English",
47+
"sourcecountry": "United States",
48+
"socialimage": "x",
49+
},
50+
{
51+
"url": "https://a.test/2",
52+
"title": "Newest B",
53+
"seendate": "20240615T100000Z",
54+
"domain": "a.test",
55+
"language": "English",
56+
"sourcecountry": "United States",
57+
},
4558
]
4659
}
4760
_GDELT_PAGE_2 = {
4861
"articles": [
49-
{"url": "https://a.test/3", "title": "Older C", "seendate": "20240614T080000Z",
50-
"domain": "a.test", "language": "English", "sourcecountry": "France"},
62+
{
63+
"url": "https://a.test/3",
64+
"title": "Older C",
65+
"seendate": "20240614T080000Z",
66+
"domain": "a.test",
67+
"language": "English",
68+
"sourcecountry": "France",
69+
},
5170
]
5271
}
5372
_NEWSAPI_ERROR = {"status": "error", "code": "apiKeyMissing", "message": "Your API key is missing."}
@@ -78,6 +97,7 @@ def do_GET(self) -> None: # noqa: N802
7897

7998

8099
def main() -> int:
100+
"""Start the mock news API and run the haybarn SQL E2E suite against it."""
81101
server = HTTPServer(("127.0.0.1", 0), _Handler)
82102
thread = threading.Thread(target=server.serve_forever, daemon=True)
83103
thread.start()

‎tests/test_mock_server.py‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,14 @@ def test_newsapi_requires_key(mock_newsapi):
8787
provider = get_provider("newsapi", base_url=mock_newsapi)
8888
try:
8989
provider.search(
90-
"x", count=2, timespan="1d", window_end=None, page=1,
91-
country=None, language=None, api_key=None,
90+
"x",
91+
count=2,
92+
timespan="1d",
93+
window_end=None,
94+
page=1,
95+
country=None,
96+
language=None,
97+
api_key=None,
9298
)
9399
raise AssertionError("expected ProviderError for missing key")
94100
except ProviderError as exc:

‎tests/test_parsers.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,7 @@ def test_gdelt_extra_json_carries_unmapped_fields():
6868

6969

7070
def test_gdelt_tone_parsed_when_present():
71-
row = gdelt.map_article(
72-
{"title": "x", "url": "u", "seendate": "20240615T120000Z", "tone": "-3.5"}
73-
)
71+
row = gdelt.map_article({"title": "x", "url": "u", "seendate": "20240615T120000Z", "tone": "-3.5"})
7472
assert row.tone == -3.5
7573

7674

‎tests/test_worker.py‎

Lines changed: 52 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,34 @@ def gdelt_paging_server():
6262
pages = [
6363
{
6464
"articles": [
65-
{"url": "https://a.test/1", "title": "Newest A", "seendate": "20240615T120000Z",
66-
"domain": "a.test", "language": "English", "sourcecountry": "United States"},
67-
{"url": "https://a.test/2", "title": "Newest B", "seendate": "20240615T100000Z",
68-
"domain": "a.test", "language": "English", "sourcecountry": "United States"},
65+
{
66+
"url": "https://a.test/1",
67+
"title": "Newest A",
68+
"seendate": "20240615T120000Z",
69+
"domain": "a.test",
70+
"language": "English",
71+
"sourcecountry": "United States",
72+
},
73+
{
74+
"url": "https://a.test/2",
75+
"title": "Newest B",
76+
"seendate": "20240615T100000Z",
77+
"domain": "a.test",
78+
"language": "English",
79+
"sourcecountry": "United States",
80+
},
6981
]
7082
},
7183
{
7284
"articles": [
73-
{"url": "https://a.test/3", "title": "Older C", "seendate": "20240614T080000Z",
74-
"domain": "a.test", "language": "English", "sourcecountry": "France"},
85+
{
86+
"url": "https://a.test/3",
87+
"title": "Older C",
88+
"seendate": "20240614T080000Z",
89+
"domain": "a.test",
90+
"language": "English",
91+
"sourcecountry": "France",
92+
},
7593
]
7694
},
7795
]
@@ -91,9 +109,7 @@ def test_news_providers_lists_providers():
91109
with _client() as client:
92110
table = _collect(client.table_function(function_name="news_providers"))
93111
assert table.column_names == ["provider", "requires_key"]
94-
providers = dict(
95-
zip(table.column("provider").to_pylist(), table.column("requires_key").to_pylist(), strict=True)
96-
)
112+
providers = dict(zip(table.column("provider").to_pylist(), table.column("requires_key").to_pylist(), strict=True))
97113
assert providers == {"gdelt": False, "newsapi": True}
98114

99115

@@ -107,7 +123,15 @@ def test_news_search_unified_schema_and_types(mock_gdelt, worker_env):
107123
)
108124
)
109125
assert table.column_names == [
110-
"title", "url", "domain", "language", "seendate", "country", "tone", "source", "extra",
126+
"title",
127+
"url",
128+
"domain",
129+
"language",
130+
"seendate",
131+
"country",
132+
"tone",
133+
"source",
134+
"extra",
111135
]
112136
# seendate is a real TIMESTAMPTZ.
113137
seendate_type = table.schema.field("seendate").type
@@ -149,30 +173,28 @@ def test_scan_state_round_trips_across_batch_boundary(gdelt_paging_server, worke
149173
def test_newsapi_path_via_mock_requires_secret(mock_newsapi, worker_env):
150174
"""Without a secret configured, the newsapi provider errors cleanly (no crash)."""
151175
worker_env({"VGI_NEWS_NEWSAPI_BASE_URL": mock_newsapi})
152-
with _client() as client:
153-
with pytest.raises(Exception) as excinfo:
154-
_collect(
155-
client.table_function(
156-
function_name="news_search",
157-
arguments=Arguments(
158-
positional=[pa.scalar("elections")],
159-
named={"provider": pa.scalar("newsapi")},
160-
),
161-
)
176+
with _client() as client, pytest.raises(Exception) as excinfo:
177+
_collect(
178+
client.table_function(
179+
function_name="news_search",
180+
arguments=Arguments(
181+
positional=[pa.scalar("elections")],
182+
named={"provider": pa.scalar("newsapi")},
183+
),
162184
)
185+
)
163186
assert "API key" in str(excinfo.value) or "secret" in str(excinfo.value).lower()
164187

165188

166189
def test_unknown_provider_errors_clean():
167-
with _client() as client:
168-
with pytest.raises(Exception) as excinfo:
169-
_collect(
170-
client.table_function(
171-
function_name="news_search",
172-
arguments=Arguments(
173-
positional=[pa.scalar("x")],
174-
named={"provider": pa.scalar("nope")},
175-
),
176-
)
190+
with _client() as client, pytest.raises(Exception) as excinfo:
191+
_collect(
192+
client.table_function(
193+
function_name="news_search",
194+
arguments=Arguments(
195+
positional=[pa.scalar("x")],
196+
named={"provider": pa.scalar("nope")},
197+
),
177198
)
199+
)
178200
assert "unknown provider" in str(excinfo.value).lower()

‎uv.lock‎

Lines changed: 24 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎vgi_news/discovery.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ class NewsProviders(TableFunctionGenerator[_NoArgs]):
5353
FIXED_SCHEMA: ClassVar[pa.Schema] = _PROVIDERS_SCHEMA
5454

5555
class Meta:
56+
"""Function metadata."""
57+
5658
name = "news_providers"
5759
description = "List available news providers and whether each requires an API key"
5860
categories = ["news", "metadata"]
@@ -65,11 +67,13 @@ class Meta:
6567

6668
@classmethod
6769
def cardinality(cls, params: BindParams[_NoArgs]) -> TableCardinality:
70+
"""Return the fixed provider-count cardinality."""
6871
n = len(provider_names())
6972
return TableCardinality(estimate=n, max=n)
7073

7174
@classmethod
7275
def process(cls, params: ProcessParams[_NoArgs], state: None, out: OutputCollector) -> None:
76+
"""Emit one row per provider with its key requirement."""
7377
names = provider_names()
7478
out.emit(
7579
pa.RecordBatch.from_pydict(

‎vgi_news/providers/__init__.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ def get_provider(name: str, *, base_url: str | None = None, timeout: float = 20.
3939
at a mock HTTP server). ``None`` uses the provider default.
4040
timeout: Per-request timeout in seconds.
4141
42+
Returns:
43+
A new provider instance for ``name``.
44+
4245
Raises:
4346
ProviderError: If ``name`` is not a known provider.
4447
"""

0 commit comments

Comments
 (0)