From 9c6adfaac7792e106d286ff282c1bd206d3403fa Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Sat, 8 Feb 2025 13:44:45 +0000 Subject: [PATCH 1/9] refactor: fallback to env vars for NearestCity.connect(), esp in tests --- pg_nearest_city/async_nearest_city.py | 18 +++++++++++++++--- pg_nearest_city/nearest_city.py | 18 +++++++++++++++--- tests/test_async_nearest_city.py | 2 +- tests/test_nearest_city.py | 2 +- 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/pg_nearest_city/async_nearest_city.py b/pg_nearest_city/async_nearest_city.py index 1be75a0..b880ba9 100644 --- a/pg_nearest_city/async_nearest_city.py +++ b/pg_nearest_city/async_nearest_city.py @@ -12,6 +12,7 @@ from pg_nearest_city import base_nearest_city from pg_nearest_city.base_nearest_city import ( BaseNearestCity, + DbConfig, InitializationStatus, Location, ) @@ -24,20 +25,31 @@ class AsyncNearestCity: @classmethod @asynccontextmanager - async def connect(cls, db: psycopg.AsyncConnection | base_nearest_city.DbConfig): + async def connect( + cls, + db: Optional[psycopg.AsyncConnection | base_nearest_city.DbConfig] = None, + ): """Managed NearestCity instance with automatic initialization and cleanup. Args: - db: Either a DbConfig for a new connection or an existing psycopg Connection + db: Either a DbConfig for a new connection or an existing + psycopg Connection. If neither are passed, attempts to extract + variables from system environment, else uses defaults. """ is_external_connection = isinstance(db, psycopg.AsyncConnection) + is_db_config = isinstance(db, base_nearest_city.DbConfig) conn: psycopg.AsyncConnection if is_external_connection: conn = db - else: + elif is_db_config: conn = await psycopg.AsyncConnection.connect(db.get_connection_string()) + else: + # Fallback to env var extraction, or defaults for testing + conn = await psycopg.AsyncConnection.connect( + DbConfig().get_connection_string() + ) geocoder = cls(conn) diff --git a/pg_nearest_city/nearest_city.py b/pg_nearest_city/nearest_city.py index b5afe6b..8459832 100644 --- a/pg_nearest_city/nearest_city.py +++ b/pg_nearest_city/nearest_city.py @@ -12,6 +12,7 @@ from pg_nearest_city import base_nearest_city from pg_nearest_city.base_nearest_city import ( BaseNearestCity, + DbConfig, InitializationStatus, Location, ) @@ -24,20 +25,31 @@ class NearestCity: @classmethod @contextmanager - def connect(cls, db: psycopg.Connection | base_nearest_city.DbConfig): + def connect( + cls, + db: Optional[psycopg.Connection | base_nearest_city.DbConfig] = None, + ): """Managed NearestCity instance with automatic initialization and cleanup. Args: - db: Either a DbConfig for a new connection or an existing psycopg Connection + db: Either a DbConfig for a new connection or an existing + psycopg Connection. If neither are passed, attempts to extract + variables from system environment, else uses defaults. """ is_external_connection = isinstance(db, psycopg.Connection) + is_db_config = isinstance(db, base_nearest_city.DbConfig) conn: psycopg.Connection if is_external_connection: conn = db - else: + elif is_db_config: conn = psycopg.Connection.connect(db.get_connection_string()) + else: + # Fallback to env var extraction, or defaults for testing + conn = psycopg.Connection.connect( + DbConfig().get_connection_string() + ) geocoder = cls(conn) diff --git a/tests/test_async_nearest_city.py b/tests/test_async_nearest_city.py index a522407..b67b75c 100644 --- a/tests/test_async_nearest_city.py +++ b/tests/test_async_nearest_city.py @@ -33,7 +33,7 @@ async def test_db(): async def test_full_initialization_connect(): """Test completet database initialization and basic query through connect method.""" - async with AsyncNearestCity.connect(get_test_config()) as geocoder: + async with AsyncNearestCity.connect() as geocoder: location = await geocoder.query(40.7128, -74.0060) assert location is not None diff --git a/tests/test_nearest_city.py b/tests/test_nearest_city.py index 5ad44b3..8b92664 100644 --- a/tests/test_nearest_city.py +++ b/tests/test_nearest_city.py @@ -33,7 +33,7 @@ def test_db(): def test_full_initialization_connect(): """Test completet database initialization and basic query through connect method.""" - with NearestCity.connect(get_test_config()) as geocoder: + with NearestCity.connect() as geocoder: location = geocoder.query(40.7128, -74.0060) assert location is not None From cdea5dfbc95ef7a1b97df6f9a0bd6d2b23ed858b Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Sun, 9 Feb 2025 18:48:06 +0000 Subject: [PATCH 2/9] refactor: use encode/httpcore unasync impl, restructure --- .pre-commit-config.yaml | 12 +- pg_nearest_city/__init__.py | 4 +- pg_nearest_city/_async/__init__.py | 1 + .../nearest_city.py} | 5 +- pg_nearest_city/_sync/__init__.py | 1 + pg_nearest_city/{ => _sync}/nearest_city.py | 11 +- pyproject.toml | 9 +- tests/_async/__init__.py | 1 + tests/_async/test_nearest_city.py | 159 ++++++++++++++++++ tests/_sync/__init__.py | 1 + tests/_sync/test_nearest_city.py | 159 ++++++++++++++++++ tests/conftest.py | 8 - unasync.py | 113 +++++++++++++ uv.lock | 35 ---- 14 files changed, 449 insertions(+), 70 deletions(-) create mode 100644 pg_nearest_city/_async/__init__.py rename pg_nearest_city/{async_nearest_city.py => _async/nearest_city.py} (98%) create mode 100644 pg_nearest_city/_sync/__init__.py rename pg_nearest_city/{ => _sync}/nearest_city.py (96%) create mode 100644 tests/_async/__init__.py create mode 100644 tests/_async/test_nearest_city.py create mode 100644 tests/_sync/__init__.py create mode 100644 tests/_sync/test_nearest_city.py create mode 100644 unasync.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 54de431..b7c91c5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,21 +1,11 @@ repos: - # Dev: install async for next command - - repo: local - hooks: - - id: dev-deps - name: instal-dev-deps - language: system - entry: uv sync --group dev - always_run: true - pass_filenames: false - # Unasync: Convert async --> sync - repo: local hooks: - id: unasync name: unasync-all language: system - entry: uv run python build_sync.py + entry: uv run python unasync.py always_run: true pass_filenames: false diff --git a/pg_nearest_city/__init__.py b/pg_nearest_city/__init__.py index 3459277..ad1c9f2 100644 --- a/pg_nearest_city/__init__.py +++ b/pg_nearest_city/__init__.py @@ -1,7 +1,7 @@ """The main pg_nearest_city package.""" -from .async_nearest_city import AsyncNearestCity +from ._async.nearest_city import AsyncNearestCity +from ._sync.nearest_city import NearestCity from .base_nearest_city import DbConfig, Location -from .nearest_city import NearestCity __all__ = ["NearestCity", "AsyncNearestCity", "DbConfig", "Location"] diff --git a/pg_nearest_city/_async/__init__.py b/pg_nearest_city/_async/__init__.py new file mode 100644 index 0000000..e82d5bb --- /dev/null +++ b/pg_nearest_city/_async/__init__.py @@ -0,0 +1 @@ +"""Async implementation.""" diff --git a/pg_nearest_city/async_nearest_city.py b/pg_nearest_city/_async/nearest_city.py similarity index 98% rename from pg_nearest_city/async_nearest_city.py rename to pg_nearest_city/_async/nearest_city.py index b880ba9..8896686 100644 --- a/pg_nearest_city/async_nearest_city.py +++ b/pg_nearest_city/_async/nearest_city.py @@ -9,7 +9,6 @@ import psycopg from psycopg import AsyncCursor -from pg_nearest_city import base_nearest_city from pg_nearest_city.base_nearest_city import ( BaseNearestCity, DbConfig, @@ -27,7 +26,7 @@ class AsyncNearestCity: @asynccontextmanager async def connect( cls, - db: Optional[psycopg.AsyncConnection | base_nearest_city.DbConfig] = None, + db: Optional[psycopg.AsyncConnection | DbConfig] = None, ): """Managed NearestCity instance with automatic initialization and cleanup. @@ -37,7 +36,7 @@ async def connect( variables from system environment, else uses defaults. """ is_external_connection = isinstance(db, psycopg.AsyncConnection) - is_db_config = isinstance(db, base_nearest_city.DbConfig) + is_db_config = isinstance(db, DbConfig) conn: psycopg.AsyncConnection diff --git a/pg_nearest_city/_sync/__init__.py b/pg_nearest_city/_sync/__init__.py new file mode 100644 index 0000000..e82d5bb --- /dev/null +++ b/pg_nearest_city/_sync/__init__.py @@ -0,0 +1 @@ +"""Async implementation.""" diff --git a/pg_nearest_city/nearest_city.py b/pg_nearest_city/_sync/nearest_city.py similarity index 96% rename from pg_nearest_city/nearest_city.py rename to pg_nearest_city/_sync/nearest_city.py index 8459832..4eb04c8 100644 --- a/pg_nearest_city/nearest_city.py +++ b/pg_nearest_city/_sync/nearest_city.py @@ -9,7 +9,6 @@ import psycopg from psycopg import Cursor -from pg_nearest_city import base_nearest_city from pg_nearest_city.base_nearest_city import ( BaseNearestCity, DbConfig, @@ -27,7 +26,7 @@ class NearestCity: @contextmanager def connect( cls, - db: Optional[psycopg.Connection | base_nearest_city.DbConfig] = None, + db: Optional[psycopg.Connection | DbConfig] = None, ): """Managed NearestCity instance with automatic initialization and cleanup. @@ -37,7 +36,7 @@ def connect( variables from system environment, else uses defaults. """ is_external_connection = isinstance(db, psycopg.Connection) - is_db_config = isinstance(db, base_nearest_city.DbConfig) + is_db_config = isinstance(db, DbConfig) conn: psycopg.Connection @@ -65,11 +64,11 @@ def __init__( connection: psycopg.Connection, logger: Optional[logging.Logger] = None, ): - """Initialize reverse geocoder with an existing AsyncConnection. + """Initialize reverse geocoder with an existing Connection. Args: - db: An existing psycopg AsyncConnection - connection: psycopg.AsyncConnection + db: An existing psycopg Connection + connection: psycopg.Connection logger: Optional custom logger. If not provided, uses package logger """ # Allow users to provide their own logger while having a sensible default diff --git a/pyproject.toml b/pyproject.toml index 5bdbea9..f167ef1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,7 +29,7 @@ documentation = "https://hotosm.github.io/pg-nearest-city" repository = "https://github.com/hotosm/pg-nearest-city" [project.scripts] -build-sync = "build_sync:build_sync" +build-sync = "unasync:main" [dependency-groups] test = [ @@ -43,9 +43,6 @@ docs = [ "mkdocs-material>=9.5.49", "mkdocstrings-python>=1.13.0", ] -dev = [ - "unasync>=0.6.0", -] [build-system] requires = ["hatchling"] @@ -70,7 +67,9 @@ exclude = [ "tests/test_nearest_city.py", ] [tool.ruff.lint] -select = ["I", "E", "W", "D", "B", "F", "N", "Q"] +# Note we do not use "I" for import sorting to avoid conflict with +# import sorting after unasync is run +select = ["E", "W", "D", "B", "F", "N", "Q"] ignore = ["N805", "B008"] [tool.ruff.lint.pydocstyle] convention = "google" diff --git a/tests/_async/__init__.py b/tests/_async/__init__.py new file mode 100644 index 0000000..fb0bcd5 --- /dev/null +++ b/tests/_async/__init__.py @@ -0,0 +1 @@ +"""Async Tests.""" diff --git a/tests/_async/test_nearest_city.py b/tests/_async/test_nearest_city.py new file mode 100644 index 0000000..b67b75c --- /dev/null +++ b/tests/_async/test_nearest_city.py @@ -0,0 +1,159 @@ +"""Test async geocoder initialization and data file loading.""" + +import psycopg +import pytest +import pytest_asyncio + +from pg_nearest_city import AsyncNearestCity, DbConfig, Location + + +def get_test_config(): + """Get database configuration from environment variables or defaults.""" + # Use default connection params + return DbConfig() + + +@pytest_asyncio.fixture() +async def test_db(): + """Provide a clean database connection for each test.""" + config = get_test_config() + + # Create a single connection for the test + conn = await psycopg.AsyncConnection.connect(config.get_connection_string()) + + # Clean up any existing state + async with conn.cursor() as cur: + await cur.execute("DROP TABLE IF EXISTS pg_nearest_city_geocoding;") + await conn.commit() + + yield conn + + await conn.close() + + +async def test_full_initialization_connect(): + """Test completet database initialization and basic query through connect method.""" + async with AsyncNearestCity.connect() as geocoder: + location = await geocoder.query(40.7128, -74.0060) + + assert location is not None + assert location.city == "New York City" + assert isinstance(location, Location) + + +async def test_full_initialization(test_db): + """Test complete database initialization and basic query.""" + geocoder = AsyncNearestCity(test_db) + await geocoder.initialize() + + # Test with New York coordinates + location = await geocoder.query(40.7128, -74.0060) + assert location is not None + assert location.city == "New York City" + assert isinstance(location, Location) + + +async def test_check_initialization_fresh_database(test_db): + """Test initialization check on a fresh database with no tables.""" + geocoder = AsyncNearestCity(test_db) + async with test_db.cursor() as cur: + status = await geocoder._check_initialization_status(cur) + + assert not status.is_fully_initialized + assert not status.has_table + + +async def test_check_initialization_incomplete_table(test_db): + """Test initialization check with a table that's missing columns.""" + geocoder = AsyncNearestCity(test_db) + + async with test_db.cursor() as cur: + await cur.execute(""" + CREATE TABLE pg_nearest_city_geocoding ( + city varchar, + country varchar + ); + """) + await test_db.commit() + + status = await geocoder._check_initialization_status(cur) + + assert not status.is_fully_initialized + assert status.has_table + assert not status.has_valid_structure + + +async def test_check_initialization_empty_table(test_db): + """Test initialization check with properly structured but empty table.""" + geocoder = AsyncNearestCity(test_db) + + async with test_db.cursor() as cur: + await geocoder._create_geocoding_table(cur) + await test_db.commit() + + status = await geocoder._check_initialization_status(cur) + + assert not status.is_fully_initialized + assert status.has_table + assert status.has_valid_structure + assert not status.has_data + + +async def test_check_initialization_missing_voronoi(test_db): + """Test initialization check when Voronoi polygons are missing.""" + geocoder = AsyncNearestCity(test_db) + + async with test_db.cursor() as cur: + await geocoder._create_geocoding_table(cur) + await geocoder._import_cities(cur) + await test_db.commit() + + status = await geocoder._check_initialization_status(cur) + + assert not status.is_fully_initialized + assert status.has_data + assert not status.has_complete_voronoi + + +async def test_check_initialization_missing_index(test_db): + """Test initialization check when spatial index is missing.""" + geocoder = AsyncNearestCity(test_db) + + async with test_db.cursor() as cur: + await geocoder._create_geocoding_table(cur) + await geocoder._import_cities(cur) + await geocoder._import_voronoi_polygons(cur) + await test_db.commit() + + status = await geocoder._check_initialization_status(cur) + + assert not status.is_fully_initialized + assert status.has_data + assert status.has_complete_voronoi + assert not status.has_spatial_index + + +async def test_check_initialization_complete(test_db): + """Test initialization check with a properly initialized database.""" + geocoder = AsyncNearestCity(test_db) + await geocoder.initialize() + + async with test_db.cursor() as cur: + status = await geocoder._check_initialization_status(cur) + + assert status.is_fully_initialized + assert status.has_spatial_index + assert status.has_complete_voronoi + assert status.has_data + + +async def test_invalid_coordinates(test_db): + """Test that invalid coordinates are properly handled.""" + geocoder = AsyncNearestCity(test_db) + await geocoder.initialize() + + with pytest.raises(ValueError): + await geocoder.query(91, 0) # Invalid latitude + + with pytest.raises(ValueError): + await geocoder.query(0, 181) # Invalid longitude diff --git a/tests/_sync/__init__.py b/tests/_sync/__init__.py new file mode 100644 index 0000000..fb0bcd5 --- /dev/null +++ b/tests/_sync/__init__.py @@ -0,0 +1 @@ +"""Async Tests.""" diff --git a/tests/_sync/test_nearest_city.py b/tests/_sync/test_nearest_city.py new file mode 100644 index 0000000..e28807b --- /dev/null +++ b/tests/_sync/test_nearest_city.py @@ -0,0 +1,159 @@ +"""Test async geocoder initialization and data file loading.""" + +import psycopg +import pytest + + +from pg_nearest_city import NearestCity, DbConfig, Location + + +def get_test_config(): + """Get database configuration from environment variables or defaults.""" + # Use default connection params + return DbConfig() + + +@pytest.fixture() +def test_db(): + """Provide a clean database connection for each test.""" + config = get_test_config() + + # Create a single connection for the test + conn = psycopg.Connection.connect(config.get_connection_string()) + + # Clean up any existing state + with conn.cursor() as cur: + cur.execute("DROP TABLE IF EXISTS pg_nearest_city_geocoding;") + conn.commit() + + yield conn + + conn.close() + + +def test_full_initialization_connect(): + """Test completet database initialization and basic query through connect method.""" + with NearestCity.connect() as geocoder: + location = geocoder.query(40.7128, -74.0060) + + assert location is not None + assert location.city == "New York City" + assert isinstance(location, Location) + + +def test_full_initialization(test_db): + """Test complete database initialization and basic query.""" + geocoder = NearestCity(test_db) + geocoder.initialize() + + # Test with New York coordinates + location = geocoder.query(40.7128, -74.0060) + assert location is not None + assert location.city == "New York City" + assert isinstance(location, Location) + + +def test_check_initialization_fresh_database(test_db): + """Test initialization check on a fresh database with no tables.""" + geocoder = NearestCity(test_db) + with test_db.cursor() as cur: + status = geocoder._check_initialization_status(cur) + + assert not status.is_fully_initialized + assert not status.has_table + + +def test_check_initialization_incomplete_table(test_db): + """Test initialization check with a table that's missing columns.""" + geocoder = NearestCity(test_db) + + with test_db.cursor() as cur: + cur.execute(""" + CREATE TABLE pg_nearest_city_geocoding ( + city varchar, + country varchar + ); + """) + test_db.commit() + + status = geocoder._check_initialization_status(cur) + + assert not status.is_fully_initialized + assert status.has_table + assert not status.has_valid_structure + + +def test_check_initialization_empty_table(test_db): + """Test initialization check with properly structured but empty table.""" + geocoder = NearestCity(test_db) + + with test_db.cursor() as cur: + geocoder._create_geocoding_table(cur) + test_db.commit() + + status = geocoder._check_initialization_status(cur) + + assert not status.is_fully_initialized + assert status.has_table + assert status.has_valid_structure + assert not status.has_data + + +def test_check_initialization_missing_voronoi(test_db): + """Test initialization check when Voronoi polygons are missing.""" + geocoder = NearestCity(test_db) + + with test_db.cursor() as cur: + geocoder._create_geocoding_table(cur) + geocoder._import_cities(cur) + test_db.commit() + + status = geocoder._check_initialization_status(cur) + + assert not status.is_fully_initialized + assert status.has_data + assert not status.has_complete_voronoi + + +def test_check_initialization_missing_index(test_db): + """Test initialization check when spatial index is missing.""" + geocoder = NearestCity(test_db) + + with test_db.cursor() as cur: + geocoder._create_geocoding_table(cur) + geocoder._import_cities(cur) + geocoder._import_voronoi_polygons(cur) + test_db.commit() + + status = geocoder._check_initialization_status(cur) + + assert not status.is_fully_initialized + assert status.has_data + assert status.has_complete_voronoi + assert not status.has_spatial_index + + +def test_check_initialization_complete(test_db): + """Test initialization check with a properly initialized database.""" + geocoder = NearestCity(test_db) + geocoder.initialize() + + with test_db.cursor() as cur: + status = geocoder._check_initialization_status(cur) + + assert status.is_fully_initialized + assert status.has_spatial_index + assert status.has_complete_voronoi + assert status.has_data + + +def test_invalid_coordinates(test_db): + """Test that invalid coordinates are properly handled.""" + geocoder = NearestCity(test_db) + geocoder.initialize() + + with pytest.raises(ValueError): + geocoder.query(91, 0) # Invalid latitude + + with pytest.raises(ValueError): + geocoder.query(0, 181) # Invalid longitude diff --git a/tests/conftest.py b/tests/conftest.py index f5d9954..18ef0e3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,9 +1 @@ """Test fixtures.""" - -import pytest - - -@pytest.fixture(scope="session") -def db(): - """Database URI.""" - return "postgresql://cities:dummycipassword@db:5432/cities" diff --git a/unasync.py b/unasync.py new file mode 100644 index 0000000..7a037c7 --- /dev/null +++ b/unasync.py @@ -0,0 +1,113 @@ +#!/usr/bin/env python + +"""Convert async modules to sync equivalents using tokenisation.""" + +import os +import re +import sys +from pprint import pprint + +SUBS = [ + # General + ("AsyncIterator", "Iterator"), + ("Async([A-Z][A-Za-z0-9_]*)", r"\2"), + ("async def", "def"), + ("async with", "with"), + ("async for", "for"), + ("await ", ""), + ("aclose", "close"), + ("aiter_stream", "iter_stream"), + ("aread", "read"), + ("asynccontextmanager", "contextmanager"), + ("__aenter__", "__enter__"), + ("__aexit__", "__exit__"), + ("__aiter__", "__iter__"), + # Package specific + ("AsyncNearestCity", "NearestCity"), + ("async_nearest_city", "nearest_city"), + ("AsyncConnection", "Connection"), + ("AsyncCursor", "Cursor"), + # Testing + ("pytest_asyncio.fixture", "pytest.fixture"), + ("import pytest_asyncio", ""), + ("pytest_asyncio", "pytest"), + ("@pytest.mark.anyio", ""), + ("@pytest.mark.asyncio", ""), +] +COMPILED_SUBS = [ + (re.compile(r"(^|\b)" + regex + r"($|\b)"), repl) for regex, repl in SUBS +] + +USED_SUBS = set() + + +def unasync_line(line): + """Remove async tokens on given line.""" + for index, (regex, repl) in enumerate(COMPILED_SUBS): + old_line = line + line = re.sub(regex, repl, line) + if old_line != line: + USED_SUBS.add(index) + return line + + +def unasync_file(in_path, out_path): + """Remove async tokens in given file.""" + with open(in_path, "r") as in_file: + with open(out_path, "w", newline="") as out_file: + for line in in_file.readlines(): + line = unasync_line(line) + out_file.write(line) + + +def unasync_file_check(in_path, out_path): + """Check if async tokens need to be removed.""" + with open(in_path, "r") as in_file: + with open(out_path, "r") as out_file: + for in_line, out_line in zip( + in_file.readlines(), out_file.readlines(), strict=False + ): + expected = unasync_line(in_line) + if out_line != expected: + print(f"unasync mismatch between {in_path!r} and {out_path!r}") + print(f"Async code: {in_line!r}") + print(f"Expected sync code: {expected!r}") + print(f"Actual sync code: {out_line!r}") + sys.exit(1) + + +def unasync_dir(in_dir, out_dir, check_only=False): + """Remove async tokens for all Python files in dir.""" + for dirpath, _dirnames, filenames in os.walk(in_dir): + for filename in filenames: + if not filename.endswith(".py"): + continue + rel_dir = os.path.relpath(dirpath, in_dir) + in_path = os.path.normpath(os.path.join(in_dir, rel_dir, filename)) + out_path = os.path.normpath(os.path.join(out_dir, rel_dir, filename)) + print(in_path, "->", out_path) + if check_only: + unasync_file_check(in_path, out_path) + else: + unasync_file(in_path, out_path) + + +def main(): + """Run the async --> sync converter.""" + check_only = "--check" in sys.argv + unasync_dir( + "pg_nearest_city/_async", "pg_nearest_city/_sync", check_only=check_only + ) + unasync_dir("tests/_async", "tests/_sync", check_only=check_only) + + if len(USED_SUBS) != len(SUBS): + unused_subs = [SUBS[i] for i in range(len(SUBS)) if i not in USED_SUBS] + + print("These patterns were not used:") + pprint(unused_subs) + # Allow pre-commit to pass + sys.exit(0) + + +if __name__ == "__main__": + main() diff --git a/uv.lock b/uv.lock index b353e97..e83b5c1 100644 --- a/uv.lock +++ b/uv.lock @@ -440,9 +440,6 @@ dependencies = [ ] [package.dev-dependencies] -dev = [ - { name = "unasync" }, -] docs = [ { name = "mkdocs" }, { name = "mkdocs-exclude" }, @@ -459,7 +456,6 @@ test = [ requires-dist = [{ name = "psycopg", specifier = ">=3.1" }] [package.metadata.requires-dev] -dev = [{ name = "unasync", specifier = ">=0.6.0" }] docs = [ { name = "mkdocs", specifier = ">=1.6.1" }, { name = "mkdocs-exclude", specifier = ">=1.0.2" }, @@ -715,15 +711,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/f9/9b/335f9764261e915ed497fcdeb11df5dfd6f7bf257d4a6a2a686d80da4d54/requests-2.32.3-py3-none-any.whl", hash = "sha256:70761cfe03c773ceb22aa2f671b4757976145175cdfca038c02654d061d6dcc6", size = 64928 }, ] -[[package]] -name = "setuptools" -version = "75.8.0" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/92/ec/089608b791d210aec4e7f97488e67ab0d33add3efccb83a056cbafe3a2a6/setuptools-75.8.0.tar.gz", hash = "sha256:c5afc8f407c626b8313a86e10311dd3f661c6cd9c09d4bf8c15c0e11f9f2b0e6", size = 1343222 } -wheels = [ - { url = "https://files.pythonhosted.org/packages/69/8a/b9dc7678803429e4a3bc9ba462fa3dd9066824d3c607490235c6a796be5a/setuptools-75.8.0-py3-none-any.whl", hash = "sha256:e3982f444617239225d675215d51f6ba05f845d4eec313da4418fdbb56fb27e3", size = 1228782 }, -] - [[package]] name = "six" version = "1.17.0" @@ -742,15 +729,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/04/be/d09147ad1ec7934636ad912901c5fd7667e1c858e19d355237db0d0cd5e4/smmap-5.0.2-py3-none-any.whl", hash = "sha256:b30115f0def7d7531d22a0fb6502488d879e75b260a9db4d0819cfb25403af5e", size = 24303 }, ] -[[package]] -name = "tokenize-rt" -version = "6.1.0" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/6b/0a/5854d8ced8c1e00193d1353d13db82d7f813f99bd5dcb776ce3e2a4c0d19/tokenize_rt-6.1.0.tar.gz", hash = "sha256:e8ee836616c0877ab7c7b54776d2fefcc3bde714449a206762425ae114b53c86", size = 5506 } -wheels = [ - { url = "https://files.pythonhosted.org/packages/87/ba/576aac29b10dfa49a6ce650001d1bb31f81e734660555eaf144bfe5b8995/tokenize_rt-6.1.0-py2.py3-none-any.whl", hash = "sha256:d706141cdec4aa5f358945abe36b911b8cbdc844545da99e811250c0cee9b6fc", size = 6015 }, -] - [[package]] name = "tomli" version = "2.2.1" @@ -808,19 +786,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/a6/ab/7e5f53c3b9d14972843a647d8d7a853969a58aecc7559cb3267302c94774/tzdata-2024.2-py2.py3-none-any.whl", hash = "sha256:a48093786cdcde33cad18c2555e8532f34422074448fbc874186f0abd79565cd", size = 346586 }, ] -[[package]] -name = "unasync" -version = "0.6.0" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "setuptools" }, - { name = "tokenize-rt" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/28/4e/735dbc0885ca197bcd80a2479ca24035627e2e768c784261fc7f1b8d7600/unasync-0.6.0.tar.gz", hash = "sha256:a9d01ace3e1068b20550ab15b7f9723b15b8bcde728bc1770bcb578374c7ee58", size = 18755 } -wheels = [ - { url = "https://files.pythonhosted.org/packages/b8/b5/d2842541718ffa12060854735587543120a31ebc339435e0bd0faf368541/unasync-0.6.0-py3-none-any.whl", hash = "sha256:9cf7aaaea9737e417d8949bf9be55dc25fdb4ef1f4edc21b58f76ff0d2b9d73f", size = 9959 }, -] - [[package]] name = "urllib3" version = "2.3.0" From 77d4ca62792aea8a1e788ac9c1e42211925ee08f Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Sun, 9 Feb 2025 22:43:01 +0000 Subject: [PATCH 3/9] fix: do not default use test db conn, error on missing vars --- pg_nearest_city/base_nearest_city.py | 19 +++++++++++---- unasync.py | 36 +++++++++++++++------------- 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/pg_nearest_city/base_nearest_city.py b/pg_nearest_city/base_nearest_city.py index 2967f74..1c84126 100644 --- a/pg_nearest_city/base_nearest_city.py +++ b/pg_nearest_city/base_nearest_city.py @@ -22,14 +22,23 @@ class DbConfig: def __post_init__(self): """Ensures env variables are read at runtime, not at class definition.""" - self.dbname = self.dbname or os.getenv("PGNEAREST_DB_NAME", "cities") - self.user = self.user or os.getenv("PGNEAREST_DB_USER", "cities") - self.password = self.password or os.getenv( - "PGNEAREST_DB_PASSWORD", "dummycipassword" - ) + self.dbname = self.dbname or os.getenv("PGNEAREST_DB_NAME") + self.user = self.user or os.getenv("PGNEAREST_DB_USER") + self.password = self.password or os.getenv("PGNEAREST_DB_PASSWORD") self.host = self.host or os.getenv("PGNEAREST_DB_HOST", "db") self.port = self.port or int(os.getenv("PGNEAREST_DB_PORT", "5432")) + # Raise error if any required field is missing + missing_fields = [ + field + for field in ["dbname", "user", "password"] + if not getattr(self, field) + ] + if missing_fields: + raise ValueError( + f"Missing required database config fields: {', '.join(missing_fields)}" + ) + def get_connection_string(self) -> str: """Connection string that psycopg accepts.""" return ( diff --git a/unasync.py b/unasync.py index 7a037c7..dcabda0 100644 --- a/unasync.py +++ b/unasync.py @@ -53,27 +53,25 @@ def unasync_line(line): def unasync_file(in_path, out_path): """Remove async tokens in given file.""" - with open(in_path, "r") as in_file: - with open(out_path, "w", newline="") as out_file: - for line in in_file.readlines(): - line = unasync_line(line) - out_file.write(line) + with open(in_path, "r") as in_file, open(out_path, "w", newline="") as out_file: + for line in in_file.readlines(): + line = unasync_line(line) + out_file.write(line) def unasync_file_check(in_path, out_path): """Check if async tokens need to be removed.""" - with open(in_path, "r") as in_file: - with open(out_path, "r") as out_file: - for in_line, out_line in zip( - in_file.readlines(), out_file.readlines(), strict=False - ): - expected = unasync_line(in_line) - if out_line != expected: - print(f"unasync mismatch between {in_path!r} and {out_path!r}") - print(f"Async code: {in_line!r}") - print(f"Expected sync code: {expected!r}") - print(f"Actual sync code: {out_line!r}") - sys.exit(1) + with open(in_path, "r") as in_file, open(out_path, "r") as out_file: + for in_line, out_line in zip( + in_file.readlines(), out_file.readlines(), strict=False + ): + expected = unasync_line(in_line) + if out_line != expected: + print(f"unasync mismatch between {in_path!r} and {out_path!r}") + print(f"Async code: {in_line!r}") + print(f"Expected sync code: {expected!r}") + print(f"Actual sync code: {out_line!r}") + sys.exit(1) def unasync_dir(in_dir, out_dir, check_only=False): @@ -95,6 +93,9 @@ def unasync_dir(in_dir, out_dir, check_only=False): def main(): """Run the async --> sync converter.""" check_only = "--check" in sys.argv + + if not check_only: + print("**Files to unasync:**") unasync_dir( "pg_nearest_city/_async", "pg_nearest_city/_sync", check_only=check_only ) @@ -103,6 +104,7 @@ def main(): if len(USED_SUBS) != len(SUBS): unused_subs = [SUBS[i] for i in range(len(SUBS)) if i not in USED_SUBS] + print("") print("These patterns were not used:") pprint(unused_subs) # Allow pre-commit to pass From 48ea3a71ba94efa870a6ddc820d939c65fd29f00 Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Sun, 9 Feb 2025 22:43:43 +0000 Subject: [PATCH 4/9] fix: add context managers via __enter__ methods, update usage --- pg_nearest_city/_async/nearest_city.py | 100 ++++++++++++++----------- pg_nearest_city/_sync/nearest_city.py | 100 ++++++++++++++----------- 2 files changed, 112 insertions(+), 88 deletions(-) diff --git a/pg_nearest_city/_async/nearest_city.py b/pg_nearest_city/_async/nearest_city.py index 8896686..22f8b7e 100644 --- a/pg_nearest_city/_async/nearest_city.py +++ b/pg_nearest_city/_async/nearest_city.py @@ -3,7 +3,6 @@ import gzip import importlib.resources import logging -from contextlib import asynccontextmanager from typing import Optional import psycopg @@ -22,46 +21,9 @@ class AsyncNearestCity: """Reverse geocoding to the nearest city over 1000 population.""" - @classmethod - @asynccontextmanager - async def connect( - cls, - db: Optional[psycopg.AsyncConnection | DbConfig] = None, - ): - """Managed NearestCity instance with automatic initialization and cleanup. - - Args: - db: Either a DbConfig for a new connection or an existing - psycopg Connection. If neither are passed, attempts to extract - variables from system environment, else uses defaults. - """ - is_external_connection = isinstance(db, psycopg.AsyncConnection) - is_db_config = isinstance(db, DbConfig) - - conn: psycopg.AsyncConnection - - if is_external_connection: - conn = db - elif is_db_config: - conn = await psycopg.AsyncConnection.connect(db.get_connection_string()) - else: - # Fallback to env var extraction, or defaults for testing - conn = await psycopg.AsyncConnection.connect( - DbConfig().get_connection_string() - ) - - geocoder = cls(conn) - - try: - await geocoder.initialize() - yield geocoder - finally: - if not is_external_connection: - await conn.close() - def __init__( self, - connection: psycopg.AsyncConnection, + db: psycopg.AsyncConnection | DbConfig | None = None, logger: Optional[logging.Logger] = None, ): """Initialize reverse geocoder with an existing AsyncConnection. @@ -69,11 +31,14 @@ def __init__( Args: db: An existing psycopg AsyncConnection connection: psycopg.AsyncConnection - logger: Optional custom logger. If not provided, uses package logger + logger: Optional custom logger. If not provided, uses package logger. """ # Allow users to provide their own logger while having a sensible default self._logger = logger or logging.getLogger("pg_nearest_city") - self.connection = connection + self._db = db + self.connection: psycopg.AsyncConnection = None + self._is_external_connection = False + self._is_initialized = False with importlib.resources.path( "pg_nearest_city.data", "cities_1000_simple.txt.gz" @@ -84,6 +49,38 @@ def __init__( ) as voronoi_path: self.voronoi_file = voronoi_path + async def __aenter__(self): + """Open the context manager.""" + self.connection = await self.get_connection(self._db) + # Create the relevant tables and validate + await self.initialize() + self._is_initialized = True + return self + + async def __aexit__(self, exc_type, exc_value, traceback): + """Close the context manager.""" + if self.connection and not self._is_external_connection: + await self.connection.close() + self._initialized = False + + async def get_connection( + self, + db: Optional[psycopg.AsyncConnection | DbConfig] = None, + ) -> psycopg.AsyncConnection: + """Determine the database connection to use.""" + self._is_external_connection = isinstance(db, psycopg.AsyncConnection) + is_db_config = isinstance(db, DbConfig) + + if self._is_external_connection: + return db + elif is_db_config: + return await psycopg.AsyncConnection.connect(db.get_connection_string()) + else: + # Fallback to env var extraction, or defaults for testing + return await psycopg.AsyncConnection.connect( + DbConfig().get_connection_string(), + ) + async def initialize(self) -> None: """Initialize the geocoding database with validation checks.""" try: @@ -137,6 +134,17 @@ async def initialize(self) -> None: self._logger.error("Database initialization failed: %s", str(e)) raise RuntimeError(f"Database initialization failed: {str(e)}") from e + def ensure_initialized(self): + """Raise an error if the context manager was not used.""" + if not self._is_initialized: + raise RuntimeError(""" + AsyncNearestCity must be used within 'async with' context. + For example: + + async with AsyncNearestCity() as geocoder: + details = geocoder.query(lat, lon) + """) + async def query(self, lat: float, lon: float) -> Optional[Location]: """Find the nearest city to the given coordinates using Voronoi regions. @@ -151,13 +159,16 @@ async def query(self, lat: float, lon: float) -> Optional[Location]: ValueError: If coordinates are out of valid ranges RuntimeError: If database query fails """ + # Throw an error if not used in 'with' block + self.ensure_initialized() + # Validate coordinate ranges BaseNearestCity.validate_coordinates(lon, lat) try: async with self.connection.cursor() as cur: await cur.execute( - BaseNearestCity._get_reverse_geocoding_query(lon, lat) + BaseNearestCity._get_reverse_geocoding_query(lon, lat), ) result = await cur.fetchone() @@ -175,7 +186,8 @@ async def query(self, lat: float, lon: float) -> Optional[Location]: raise RuntimeError(f"Reverse geocoding failed: {str(e)}") from e async def _check_initialization_status( - self, cur: psycopg.AsyncCursor + self, + cur: psycopg.AsyncCursor, ) -> InitializationStatus: """Check the status and integrity of the geocoding database. @@ -270,7 +282,7 @@ async def _import_voronoi_polygons(self, cur: AsyncCursor): # Import the binary WKB data async with cur.copy( - "COPY voronoi_import (city, country, wkb) FROM STDIN" + "COPY voronoi_import (city, country, wkb) FROM STDIN", ) as copy: with gzip.open(self.voronoi_file, "rb") as f: while data := f.read(8192): diff --git a/pg_nearest_city/_sync/nearest_city.py b/pg_nearest_city/_sync/nearest_city.py index 4eb04c8..5dd0e65 100644 --- a/pg_nearest_city/_sync/nearest_city.py +++ b/pg_nearest_city/_sync/nearest_city.py @@ -3,7 +3,6 @@ import gzip import importlib.resources import logging -from contextlib import contextmanager from typing import Optional import psycopg @@ -22,46 +21,9 @@ class NearestCity: """Reverse geocoding to the nearest city over 1000 population.""" - @classmethod - @contextmanager - def connect( - cls, - db: Optional[psycopg.Connection | DbConfig] = None, - ): - """Managed NearestCity instance with automatic initialization and cleanup. - - Args: - db: Either a DbConfig for a new connection or an existing - psycopg Connection. If neither are passed, attempts to extract - variables from system environment, else uses defaults. - """ - is_external_connection = isinstance(db, psycopg.Connection) - is_db_config = isinstance(db, DbConfig) - - conn: psycopg.Connection - - if is_external_connection: - conn = db - elif is_db_config: - conn = psycopg.Connection.connect(db.get_connection_string()) - else: - # Fallback to env var extraction, or defaults for testing - conn = psycopg.Connection.connect( - DbConfig().get_connection_string() - ) - - geocoder = cls(conn) - - try: - geocoder.initialize() - yield geocoder - finally: - if not is_external_connection: - conn.close() - def __init__( self, - connection: psycopg.Connection, + db: psycopg.Connection | DbConfig | None = None, logger: Optional[logging.Logger] = None, ): """Initialize reverse geocoder with an existing Connection. @@ -69,11 +31,14 @@ def __init__( Args: db: An existing psycopg Connection connection: psycopg.Connection - logger: Optional custom logger. If not provided, uses package logger + logger: Optional custom logger. If not provided, uses package logger. """ # Allow users to provide their own logger while having a sensible default self._logger = logger or logging.getLogger("pg_nearest_city") - self.connection = connection + self._db = db + self.connection: psycopg.Connection = None + self._is_external_connection = False + self._is_initialized = False with importlib.resources.path( "pg_nearest_city.data", "cities_1000_simple.txt.gz" @@ -84,6 +49,38 @@ def __init__( ) as voronoi_path: self.voronoi_file = voronoi_path + def __enter__(self): + """Open the context manager.""" + self.connection = self.get_connection(self._db) + # Create the relevant tables and validate + self.initialize() + self._is_initialized = True + return self + + def __exit__(self, exc_type, exc_value, traceback): + """Close the context manager.""" + if self.connection and not self._is_external_connection: + self.connection.close() + self._initialized = False + + def get_connection( + self, + db: Optional[psycopg.Connection | DbConfig] = None, + ) -> psycopg.Connection: + """Determine the database connection to use.""" + self._is_external_connection = isinstance(db, psycopg.Connection) + is_db_config = isinstance(db, DbConfig) + + if self._is_external_connection: + return db + elif is_db_config: + return psycopg.Connection.connect(db.get_connection_string()) + else: + # Fallback to env var extraction, or defaults for testing + return psycopg.Connection.connect( + DbConfig().get_connection_string(), + ) + def initialize(self) -> None: """Initialize the geocoding database with validation checks.""" try: @@ -137,6 +134,17 @@ def initialize(self) -> None: self._logger.error("Database initialization failed: %s", str(e)) raise RuntimeError(f"Database initialization failed: {str(e)}") from e + def ensure_initialized(self): + """Raise an error if the context manager was not used.""" + if not self._is_initialized: + raise RuntimeError(""" + NearestCity must be used within 'with' context. + For example: + + with NearestCity() as geocoder: + details = geocoder.query(lat, lon) + """) + def query(self, lat: float, lon: float) -> Optional[Location]: """Find the nearest city to the given coordinates using Voronoi regions. @@ -151,13 +159,16 @@ def query(self, lat: float, lon: float) -> Optional[Location]: ValueError: If coordinates are out of valid ranges RuntimeError: If database query fails """ + # Throw an error if not used in 'with' block + self.ensure_initialized() + # Validate coordinate ranges BaseNearestCity.validate_coordinates(lon, lat) try: with self.connection.cursor() as cur: cur.execute( - BaseNearestCity._get_reverse_geocoding_query(lon, lat) + BaseNearestCity._get_reverse_geocoding_query(lon, lat), ) result = cur.fetchone() @@ -175,7 +186,8 @@ def query(self, lat: float, lon: float) -> Optional[Location]: raise RuntimeError(f"Reverse geocoding failed: {str(e)}") from e def _check_initialization_status( - self, cur: psycopg.Cursor + self, + cur: psycopg.Cursor, ) -> InitializationStatus: """Check the status and integrity of the geocoding database. @@ -270,7 +282,7 @@ def _import_voronoi_polygons(self, cur: Cursor): # Import the binary WKB data with cur.copy( - "COPY voronoi_import (city, country, wkb) FROM STDIN" + "COPY voronoi_import (city, country, wkb) FROM STDIN", ) as copy: with gzip.open(self.voronoi_file, "rb") as f: while data := f.read(8192): From bfa371c10dc54298cab586244e1eef0092ed8792 Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Sun, 9 Feb 2025 22:44:01 +0000 Subject: [PATCH 5/9] docs: update readme usage examples to remove requirement for .connect() --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 5e6c11e..6f14b6e 100644 --- a/README.md +++ b/README.md @@ -106,7 +106,7 @@ from pg_nearest_city import AsyncNearestCity # Existing code to get db connection, say from API endpoint db = await get_db_connection() -async with AsyncNearestCity.connect(db) as geocoder: +async with AsyncNearestCity(db) as geocoder: location = await geocoder.query(40.7128, -74.0060) print(location.city) @@ -123,7 +123,7 @@ from pg_nearest_city import NearestCity # Existing code to get db connection, say from API endpoint db = get_db_connection() -with AsyncNearestCity.connect(db) as geocoder: +with NearestCity(db) as geocoder: location = geocoder.query(40.7128, -74.0060) print(location.city) @@ -150,7 +150,7 @@ db_config = DbConfig( port="5432", ) -async with AsyncNearestCity.connect(db_config) as geocoder: +async with AsyncNearestCity(db_config) as geocoder: location = await geocoder.query(40.7128, -74.0060) ``` @@ -167,9 +167,9 @@ PGNEAREST_DB_PORT=5432 then ```python -from pg_nearest_city import DbConfig, AsyncNearestCity +from pg_nearest_city import AsyncNearestCity -async with AsyncNearestCity.connect() as geocoder: +async with AsyncNearestCity() as geocoder: location = await geocoder.query(40.7128, -74.0060) ``` From 69a7c518ebf43a2adf97fbc4677a22db11d22083 Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Sun, 9 Feb 2025 22:44:25 +0000 Subject: [PATCH 6/9] test: update tests to reflect changes --- compose.yml | 5 + tests/_async/test_nearest_city.py | 75 ++++++++------ tests/_sync/test_nearest_city.py | 75 ++++++++------ tests/conftest.py | 11 +++ tests/test_async_nearest_city.py | 159 ------------------------------ tests/test_nearest_city.py | 159 ------------------------------ 6 files changed, 102 insertions(+), 382 deletions(-) delete mode 100644 tests/test_async_nearest_city.py delete mode 100644 tests/test_nearest_city.py diff --git a/compose.yml b/compose.yml index 8dc217b..d5ceb2c 100644 --- a/compose.yml +++ b/compose.yml @@ -32,6 +32,11 @@ services: - ./pg_nearest_city:/opt/python/lib/python3.10/site-packages/pg_nearest_city:ro # Mount local tests - ./tests:/data/tests:ro + environment: + - PGNEAREST_DB_HOST=db + - PGNEAREST_DB_USER=cities + - PGNEAREST_DB_PASSWORD=dummycipassword + - PGNEAREST_DB_NAME=cities depends_on: db: condition: service_healthy diff --git a/tests/_async/test_nearest_city.py b/tests/_async/test_nearest_city.py index b67b75c..e2f4948 100644 --- a/tests/_async/test_nearest_city.py +++ b/tests/_async/test_nearest_city.py @@ -1,25 +1,21 @@ """Test async geocoder initialization and data file loading.""" -import psycopg +import os + import pytest import pytest_asyncio +import psycopg -from pg_nearest_city import AsyncNearestCity, DbConfig, Location - - -def get_test_config(): - """Get database configuration from environment variables or defaults.""" - # Use default connection params - return DbConfig() +from pg_nearest_city import AsyncNearestCity, Location, DbConfig +# NOTE we define the fixture here and not in conftest.py to allow +# async --> sync conversion to take place @pytest_asyncio.fixture() -async def test_db(): +async def test_db(test_db_conn_string): """Provide a clean database connection for each test.""" - config = get_test_config() - # Create a single connection for the test - conn = await psycopg.AsyncConnection.connect(config.get_connection_string()) + conn = await psycopg.AsyncConnection.connect(test_db_conn_string) # Clean up any existing state async with conn.cursor() as cur: @@ -31,9 +27,28 @@ async def test_db(): await conn.close() -async def test_full_initialization_connect(): +async def test_db_conn_missng_vars(): + """Check db connection error raised on missing vars.""" + os.environ["PGNEAREST_DB_USER"] = "" + os.environ["PGNEAREST_DB_PASSWORD"] = "" + + with pytest.raises(ValueError): + DbConfig() + + +async def test_db_conn_vars_from_env(): + """Check db connection variables are passed through.""" + db_conf = DbConfig() + assert db_conf.host == os.getenv("PGNEAREST_DB_HOST") + assert db_conf.user == os.getenv("PGNEAREST_DB_USER") + assert db_conf.password == os.getenv("PGNEAREST_DB_PASSWORD") + assert db_conf.dbname == os.getenv("PGNEAREST_DB_NAME") + assert db_conf.port == 5432 + + +async def test_full_initialization_query(): """Test completet database initialization and basic query through connect method.""" - async with AsyncNearestCity.connect() as geocoder: + async with AsyncNearestCity() as geocoder: location = await geocoder.query(40.7128, -74.0060) assert location is not None @@ -41,21 +56,17 @@ async def test_full_initialization_connect(): assert isinstance(location, Location) -async def test_full_initialization(test_db): - """Test complete database initialization and basic query.""" - geocoder = AsyncNearestCity(test_db) - await geocoder.initialize() - - # Test with New York coordinates - location = await geocoder.query(40.7128, -74.0060) - assert location is not None - assert location.city == "New York City" - assert isinstance(location, Location) +async def test_init_without_context_manager(): + """Should raise an error if not used in with block.""" + with pytest.raises(RuntimeError): + geocoder = AsyncNearestCity() + await geocoder.query(40.7128, -74.0060) async def test_check_initialization_fresh_database(test_db): """Test initialization check on a fresh database with no tables.""" geocoder = AsyncNearestCity(test_db) + async with test_db.cursor() as cur: status = await geocoder._check_initialization_status(cur) @@ -135,8 +146,8 @@ async def test_check_initialization_missing_index(test_db): async def test_check_initialization_complete(test_db): """Test initialization check with a properly initialized database.""" - geocoder = AsyncNearestCity(test_db) - await geocoder.initialize() + async with AsyncNearestCity(test_db) as geocoder: + await geocoder.initialize() async with test_db.cursor() as cur: status = await geocoder._check_initialization_status(cur) @@ -149,11 +160,11 @@ async def test_check_initialization_complete(test_db): async def test_invalid_coordinates(test_db): """Test that invalid coordinates are properly handled.""" - geocoder = AsyncNearestCity(test_db) - await geocoder.initialize() + async with AsyncNearestCity(test_db) as geocoder: + await geocoder.initialize() - with pytest.raises(ValueError): - await geocoder.query(91, 0) # Invalid latitude + with pytest.raises(ValueError): + await geocoder.query(91, 0) # Invalid latitude - with pytest.raises(ValueError): - await geocoder.query(0, 181) # Invalid longitude + with pytest.raises(ValueError): + await geocoder.query(0, 181) # Invalid longitude diff --git a/tests/_sync/test_nearest_city.py b/tests/_sync/test_nearest_city.py index e28807b..3e2c614 100644 --- a/tests/_sync/test_nearest_city.py +++ b/tests/_sync/test_nearest_city.py @@ -1,25 +1,21 @@ """Test async geocoder initialization and data file loading.""" -import psycopg -import pytest - +import os -from pg_nearest_city import NearestCity, DbConfig, Location +import pytest +import psycopg -def get_test_config(): - """Get database configuration from environment variables or defaults.""" - # Use default connection params - return DbConfig() +from pg_nearest_city import NearestCity, Location, DbConfig +# NOTE we define the fixture here and not in conftest.py to allow +# async --> sync conversion to take place @pytest.fixture() -def test_db(): +def test_db(test_db_conn_string): """Provide a clean database connection for each test.""" - config = get_test_config() - # Create a single connection for the test - conn = psycopg.Connection.connect(config.get_connection_string()) + conn = psycopg.Connection.connect(test_db_conn_string) # Clean up any existing state with conn.cursor() as cur: @@ -31,9 +27,28 @@ def test_db(): conn.close() -def test_full_initialization_connect(): +def test_db_conn_missng_vars(): + """Check db connection error raised on missing vars.""" + os.environ["PGNEAREST_DB_USER"] = "" + os.environ["PGNEAREST_DB_PASSWORD"] = "" + + with pytest.raises(ValueError): + DbConfig() + + +def test_db_conn_vars_from_env(): + """Check db connection variables are passed through.""" + db_conf = DbConfig() + assert db_conf.host == os.getenv("PGNEAREST_DB_HOST") + assert db_conf.user == os.getenv("PGNEAREST_DB_USER") + assert db_conf.password == os.getenv("PGNEAREST_DB_PASSWORD") + assert db_conf.dbname == os.getenv("PGNEAREST_DB_NAME") + assert db_conf.port == 5432 + + +def test_full_initialization_query(): """Test completet database initialization and basic query through connect method.""" - with NearestCity.connect() as geocoder: + with NearestCity() as geocoder: location = geocoder.query(40.7128, -74.0060) assert location is not None @@ -41,21 +56,17 @@ def test_full_initialization_connect(): assert isinstance(location, Location) -def test_full_initialization(test_db): - """Test complete database initialization and basic query.""" - geocoder = NearestCity(test_db) - geocoder.initialize() - - # Test with New York coordinates - location = geocoder.query(40.7128, -74.0060) - assert location is not None - assert location.city == "New York City" - assert isinstance(location, Location) +def test_init_without_context_manager(): + """Should raise an error if not used in with block.""" + with pytest.raises(RuntimeError): + geocoder = NearestCity() + geocoder.query(40.7128, -74.0060) def test_check_initialization_fresh_database(test_db): """Test initialization check on a fresh database with no tables.""" geocoder = NearestCity(test_db) + with test_db.cursor() as cur: status = geocoder._check_initialization_status(cur) @@ -135,8 +146,8 @@ def test_check_initialization_missing_index(test_db): def test_check_initialization_complete(test_db): """Test initialization check with a properly initialized database.""" - geocoder = NearestCity(test_db) - geocoder.initialize() + with NearestCity(test_db) as geocoder: + geocoder.initialize() with test_db.cursor() as cur: status = geocoder._check_initialization_status(cur) @@ -149,11 +160,11 @@ def test_check_initialization_complete(test_db): def test_invalid_coordinates(test_db): """Test that invalid coordinates are properly handled.""" - geocoder = NearestCity(test_db) - geocoder.initialize() + with NearestCity(test_db) as geocoder: + geocoder.initialize() - with pytest.raises(ValueError): - geocoder.query(91, 0) # Invalid latitude + with pytest.raises(ValueError): + geocoder.query(91, 0) # Invalid latitude - with pytest.raises(ValueError): - geocoder.query(0, 181) # Invalid longitude + with pytest.raises(ValueError): + geocoder.query(0, 181) # Invalid longitude diff --git a/tests/conftest.py b/tests/conftest.py index 18ef0e3..eb90c28 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1 +1,12 @@ """Test fixtures.""" + +import pytest + +from pg_nearest_city import DbConfig + + +@pytest.fixture() +def test_db_conn_string(): + """Get the database connection string for the test db.""" + # Use connection params from env + return DbConfig().get_connection_string() diff --git a/tests/test_async_nearest_city.py b/tests/test_async_nearest_city.py deleted file mode 100644 index b67b75c..0000000 --- a/tests/test_async_nearest_city.py +++ /dev/null @@ -1,159 +0,0 @@ -"""Test async geocoder initialization and data file loading.""" - -import psycopg -import pytest -import pytest_asyncio - -from pg_nearest_city import AsyncNearestCity, DbConfig, Location - - -def get_test_config(): - """Get database configuration from environment variables or defaults.""" - # Use default connection params - return DbConfig() - - -@pytest_asyncio.fixture() -async def test_db(): - """Provide a clean database connection for each test.""" - config = get_test_config() - - # Create a single connection for the test - conn = await psycopg.AsyncConnection.connect(config.get_connection_string()) - - # Clean up any existing state - async with conn.cursor() as cur: - await cur.execute("DROP TABLE IF EXISTS pg_nearest_city_geocoding;") - await conn.commit() - - yield conn - - await conn.close() - - -async def test_full_initialization_connect(): - """Test completet database initialization and basic query through connect method.""" - async with AsyncNearestCity.connect() as geocoder: - location = await geocoder.query(40.7128, -74.0060) - - assert location is not None - assert location.city == "New York City" - assert isinstance(location, Location) - - -async def test_full_initialization(test_db): - """Test complete database initialization and basic query.""" - geocoder = AsyncNearestCity(test_db) - await geocoder.initialize() - - # Test with New York coordinates - location = await geocoder.query(40.7128, -74.0060) - assert location is not None - assert location.city == "New York City" - assert isinstance(location, Location) - - -async def test_check_initialization_fresh_database(test_db): - """Test initialization check on a fresh database with no tables.""" - geocoder = AsyncNearestCity(test_db) - async with test_db.cursor() as cur: - status = await geocoder._check_initialization_status(cur) - - assert not status.is_fully_initialized - assert not status.has_table - - -async def test_check_initialization_incomplete_table(test_db): - """Test initialization check with a table that's missing columns.""" - geocoder = AsyncNearestCity(test_db) - - async with test_db.cursor() as cur: - await cur.execute(""" - CREATE TABLE pg_nearest_city_geocoding ( - city varchar, - country varchar - ); - """) - await test_db.commit() - - status = await geocoder._check_initialization_status(cur) - - assert not status.is_fully_initialized - assert status.has_table - assert not status.has_valid_structure - - -async def test_check_initialization_empty_table(test_db): - """Test initialization check with properly structured but empty table.""" - geocoder = AsyncNearestCity(test_db) - - async with test_db.cursor() as cur: - await geocoder._create_geocoding_table(cur) - await test_db.commit() - - status = await geocoder._check_initialization_status(cur) - - assert not status.is_fully_initialized - assert status.has_table - assert status.has_valid_structure - assert not status.has_data - - -async def test_check_initialization_missing_voronoi(test_db): - """Test initialization check when Voronoi polygons are missing.""" - geocoder = AsyncNearestCity(test_db) - - async with test_db.cursor() as cur: - await geocoder._create_geocoding_table(cur) - await geocoder._import_cities(cur) - await test_db.commit() - - status = await geocoder._check_initialization_status(cur) - - assert not status.is_fully_initialized - assert status.has_data - assert not status.has_complete_voronoi - - -async def test_check_initialization_missing_index(test_db): - """Test initialization check when spatial index is missing.""" - geocoder = AsyncNearestCity(test_db) - - async with test_db.cursor() as cur: - await geocoder._create_geocoding_table(cur) - await geocoder._import_cities(cur) - await geocoder._import_voronoi_polygons(cur) - await test_db.commit() - - status = await geocoder._check_initialization_status(cur) - - assert not status.is_fully_initialized - assert status.has_data - assert status.has_complete_voronoi - assert not status.has_spatial_index - - -async def test_check_initialization_complete(test_db): - """Test initialization check with a properly initialized database.""" - geocoder = AsyncNearestCity(test_db) - await geocoder.initialize() - - async with test_db.cursor() as cur: - status = await geocoder._check_initialization_status(cur) - - assert status.is_fully_initialized - assert status.has_spatial_index - assert status.has_complete_voronoi - assert status.has_data - - -async def test_invalid_coordinates(test_db): - """Test that invalid coordinates are properly handled.""" - geocoder = AsyncNearestCity(test_db) - await geocoder.initialize() - - with pytest.raises(ValueError): - await geocoder.query(91, 0) # Invalid latitude - - with pytest.raises(ValueError): - await geocoder.query(0, 181) # Invalid longitude diff --git a/tests/test_nearest_city.py b/tests/test_nearest_city.py deleted file mode 100644 index 8b92664..0000000 --- a/tests/test_nearest_city.py +++ /dev/null @@ -1,159 +0,0 @@ -"""Test async geocoder initialization and data file loading.""" - -import psycopg -import pytest -import pytest - -from pg_nearest_city import NearestCity, DbConfig, Location - - -def get_test_config(): - """Get database configuration from environment variables or defaults.""" - # Use default connection params - return DbConfig() - - -@pytest.fixture() -def test_db(): - """Provide a clean database connection for each test.""" - config = get_test_config() - - # Create a single connection for the test - conn = psycopg.Connection.connect(config.get_connection_string()) - - # Clean up any existing state - with conn.cursor() as cur: - cur.execute("DROP TABLE IF EXISTS pg_nearest_city_geocoding;") - conn.commit() - - yield conn - - conn.close() - - -def test_full_initialization_connect(): - """Test completet database initialization and basic query through connect method.""" - with NearestCity.connect() as geocoder: - location = geocoder.query(40.7128, -74.0060) - - assert location is not None - assert location.city == "New York City" - assert isinstance(location, Location) - - -def test_full_initialization(test_db): - """Test complete database initialization and basic query.""" - geocoder = NearestCity(test_db) - geocoder.initialize() - - # Test with New York coordinates - location = geocoder.query(40.7128, -74.0060) - assert location is not None - assert location.city == "New York City" - assert isinstance(location, Location) - - -def test_check_initialization_fresh_database(test_db): - """Test initialization check on a fresh database with no tables.""" - geocoder = NearestCity(test_db) - with test_db.cursor() as cur: - status = geocoder._check_initialization_status(cur) - - assert not status.is_fully_initialized - assert not status.has_table - - -def test_check_initialization_incomplete_table(test_db): - """Test initialization check with a table that's missing columns.""" - geocoder = NearestCity(test_db) - - with test_db.cursor() as cur: - cur.execute(""" - CREATE TABLE pg_nearest_city_geocoding ( - city varchar, - country varchar - ); - """) - test_db.commit() - - status = geocoder._check_initialization_status(cur) - - assert not status.is_fully_initialized - assert status.has_table - assert not status.has_valid_structure - - -def test_check_initialization_empty_table(test_db): - """Test initialization check with properly structured but empty table.""" - geocoder = NearestCity(test_db) - - with test_db.cursor() as cur: - geocoder._create_geocoding_table(cur) - test_db.commit() - - status = geocoder._check_initialization_status(cur) - - assert not status.is_fully_initialized - assert status.has_table - assert status.has_valid_structure - assert not status.has_data - - -def test_check_initialization_missing_voronoi(test_db): - """Test initialization check when Voronoi polygons are missing.""" - geocoder = NearestCity(test_db) - - with test_db.cursor() as cur: - geocoder._create_geocoding_table(cur) - geocoder._import_cities(cur) - test_db.commit() - - status = geocoder._check_initialization_status(cur) - - assert not status.is_fully_initialized - assert status.has_data - assert not status.has_complete_voronoi - - -def test_check_initialization_missing_index(test_db): - """Test initialization check when spatial index is missing.""" - geocoder = NearestCity(test_db) - - with test_db.cursor() as cur: - geocoder._create_geocoding_table(cur) - geocoder._import_cities(cur) - geocoder._import_voronoi_polygons(cur) - test_db.commit() - - status = geocoder._check_initialization_status(cur) - - assert not status.is_fully_initialized - assert status.has_data - assert status.has_complete_voronoi - assert not status.has_spatial_index - - -def test_check_initialization_complete(test_db): - """Test initialization check with a properly initialized database.""" - geocoder = NearestCity(test_db) - geocoder.initialize() - - with test_db.cursor() as cur: - status = geocoder._check_initialization_status(cur) - - assert status.is_fully_initialized - assert status.has_spatial_index - assert status.has_complete_voronoi - assert status.has_data - - -def test_invalid_coordinates(test_db): - """Test that invalid coordinates are properly handled.""" - geocoder = NearestCity(test_db) - geocoder.initialize() - - with pytest.raises(ValueError): - geocoder.query(91, 0) # Invalid latitude - - with pytest.raises(ValueError): - geocoder.query(0, 181) # Invalid longitude From b36054c23143fe42e007a626a3ae7226ba3c1c6b Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Sun, 9 Feb 2025 23:02:44 +0000 Subject: [PATCH 7/9] test: add test for double initialisation --- pg_nearest_city/_async/nearest_city.py | 23 +++++++++++++++-------- pg_nearest_city/_sync/nearest_city.py | 23 +++++++++++++++-------- tests/_async/test_nearest_city.py | 13 +++++++++++++ tests/_sync/test_nearest_city.py | 13 +++++++++++++ 4 files changed, 56 insertions(+), 16 deletions(-) diff --git a/pg_nearest_city/_async/nearest_city.py b/pg_nearest_city/_async/nearest_city.py index 22f8b7e..a7f2fdc 100644 --- a/pg_nearest_city/_async/nearest_city.py +++ b/pg_nearest_city/_async/nearest_city.py @@ -4,6 +4,7 @@ import importlib.resources import logging from typing import Optional +from textwrap import dedent, fill import psycopg from psycopg import AsyncCursor @@ -83,6 +84,9 @@ async def get_connection( async def initialize(self) -> None: """Initialize the geocoding database with validation checks.""" + if not self.connection: + self._inform_user_if_not_context_manager() + try: async with self.connection.cursor() as cur: self._logger.info("Starting database initialization check") @@ -134,16 +138,19 @@ async def initialize(self) -> None: self._logger.error("Database initialization failed: %s", str(e)) raise RuntimeError(f"Database initialization failed: {str(e)}") from e - def ensure_initialized(self): + def _inform_user_if_not_context_manager(self): """Raise an error if the context manager was not used.""" if not self._is_initialized: - raise RuntimeError(""" - AsyncNearestCity must be used within 'async with' context. - For example: - - async with AsyncNearestCity() as geocoder: - details = geocoder.query(lat, lon) + raise RuntimeError( + fill( + dedent(""" + AsyncNearestCity must be used within 'async with' context.\n + For example:\n + async with AsyncNearestCity() as geocoder:\n + details = geocoder.query(lat, lon) """) + ) + ) async def query(self, lat: float, lon: float) -> Optional[Location]: """Find the nearest city to the given coordinates using Voronoi regions. @@ -160,7 +167,7 @@ async def query(self, lat: float, lon: float) -> Optional[Location]: RuntimeError: If database query fails """ # Throw an error if not used in 'with' block - self.ensure_initialized() + self._inform_user_if_not_context_manager() # Validate coordinate ranges BaseNearestCity.validate_coordinates(lon, lat) diff --git a/pg_nearest_city/_sync/nearest_city.py b/pg_nearest_city/_sync/nearest_city.py index 5dd0e65..f0d0a99 100644 --- a/pg_nearest_city/_sync/nearest_city.py +++ b/pg_nearest_city/_sync/nearest_city.py @@ -4,6 +4,7 @@ import importlib.resources import logging from typing import Optional +from textwrap import dedent, fill import psycopg from psycopg import Cursor @@ -83,6 +84,9 @@ def get_connection( def initialize(self) -> None: """Initialize the geocoding database with validation checks.""" + if not self.connection: + self._inform_user_if_not_context_manager() + try: with self.connection.cursor() as cur: self._logger.info("Starting database initialization check") @@ -134,16 +138,19 @@ def initialize(self) -> None: self._logger.error("Database initialization failed: %s", str(e)) raise RuntimeError(f"Database initialization failed: {str(e)}") from e - def ensure_initialized(self): + def _inform_user_if_not_context_manager(self): """Raise an error if the context manager was not used.""" if not self._is_initialized: - raise RuntimeError(""" - NearestCity must be used within 'with' context. - For example: - - with NearestCity() as geocoder: - details = geocoder.query(lat, lon) + raise RuntimeError( + fill( + dedent(""" + NearestCity must be used within 'with' context.\n + For example:\n + with NearestCity() as geocoder:\n + details = geocoder.query(lat, lon) """) + ) + ) def query(self, lat: float, lon: float) -> Optional[Location]: """Find the nearest city to the given coordinates using Voronoi regions. @@ -160,7 +167,7 @@ def query(self, lat: float, lon: float) -> Optional[Location]: RuntimeError: If database query fails """ # Throw an error if not used in 'with' block - self.ensure_initialized() + self._inform_user_if_not_context_manager() # Validate coordinate ranges BaseNearestCity.validate_coordinates(lon, lat) diff --git a/tests/_async/test_nearest_city.py b/tests/_async/test_nearest_city.py index e2f4948..385734a 100644 --- a/tests/_async/test_nearest_city.py +++ b/tests/_async/test_nearest_city.py @@ -158,6 +158,19 @@ async def test_check_initialization_complete(test_db): assert status.has_data +async def test_init_db_at_startup_then_query(test_db): + """Web servers have a startup lifecycle that could do the initialisation.""" + async with AsyncNearestCity(test_db) as geocoder: + pass # do nothing, initialisation is complete here + + async with AsyncNearestCity() as geocoder: + location = await geocoder.query(40.7128, -74.0060) + + assert location is not None + assert location.city == "New York City" + assert isinstance(location, Location) + + async def test_invalid_coordinates(test_db): """Test that invalid coordinates are properly handled.""" async with AsyncNearestCity(test_db) as geocoder: diff --git a/tests/_sync/test_nearest_city.py b/tests/_sync/test_nearest_city.py index 3e2c614..2686fec 100644 --- a/tests/_sync/test_nearest_city.py +++ b/tests/_sync/test_nearest_city.py @@ -158,6 +158,19 @@ def test_check_initialization_complete(test_db): assert status.has_data +def test_init_db_at_startup_then_query(test_db): + """Web servers have a startup lifecycle that could do the initialisation.""" + with NearestCity(test_db) as geocoder: + pass # do nothing, initialisation is complete here + + with NearestCity() as geocoder: + location = geocoder.query(40.7128, -74.0060) + + assert location is not None + assert location.city == "New York City" + assert isinstance(location, Location) + + def test_invalid_coordinates(test_db): """Test that invalid coordinates are properly handled.""" with NearestCity(test_db) as geocoder: From fc44bed9aa5eb29184f591eb543fb5aec98734a2 Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Sun, 9 Feb 2025 23:06:01 +0000 Subject: [PATCH 8/9] test: re-add env vars after test --- tests/_async/test_nearest_city.py | 7 +++++++ tests/_sync/test_nearest_city.py | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/tests/_async/test_nearest_city.py b/tests/_async/test_nearest_city.py index 385734a..185f629 100644 --- a/tests/_async/test_nearest_city.py +++ b/tests/_async/test_nearest_city.py @@ -29,12 +29,19 @@ async def test_db(test_db_conn_string): async def test_db_conn_missng_vars(): """Check db connection error raised on missing vars.""" + original_user = os.getenv("PGNEAREST_DB_USER") + original_pass = os.getenv("PGNEAREST_DB_PASSWORD") + os.environ["PGNEAREST_DB_USER"] = "" os.environ["PGNEAREST_DB_PASSWORD"] = "" with pytest.raises(ValueError): DbConfig() + # Re-set env vars, so following tests dont fail + os.environ["PGNEAREST_DB_USER"] = original_user or "" + os.environ["PGNEAREST_DB_PASSWORD"] = original_pass or "" + async def test_db_conn_vars_from_env(): """Check db connection variables are passed through.""" diff --git a/tests/_sync/test_nearest_city.py b/tests/_sync/test_nearest_city.py index 2686fec..2ab6f05 100644 --- a/tests/_sync/test_nearest_city.py +++ b/tests/_sync/test_nearest_city.py @@ -29,12 +29,19 @@ def test_db(test_db_conn_string): def test_db_conn_missng_vars(): """Check db connection error raised on missing vars.""" + original_user = os.getenv("PGNEAREST_DB_USER") + original_pass = os.getenv("PGNEAREST_DB_PASSWORD") + os.environ["PGNEAREST_DB_USER"] = "" os.environ["PGNEAREST_DB_PASSWORD"] = "" with pytest.raises(ValueError): DbConfig() + # Re-set env vars, so following tests dont fail + os.environ["PGNEAREST_DB_USER"] = original_user or "" + os.environ["PGNEAREST_DB_PASSWORD"] = original_pass or "" + def test_db_conn_vars_from_env(): """Check db connection variables are passed through.""" From 2e70f1d44a48fb12b56fe3e370851d705b284d5c Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Sun, 9 Feb 2025 23:11:54 +0000 Subject: [PATCH 9/9] ci: remove requirement for uv + deps in pre-commit unasync (pure python) --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b7c91c5..ee54ea0 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,7 +5,7 @@ repos: - id: unasync name: unasync-all language: system - entry: uv run python unasync.py + entry: python unasync.py always_run: true pass_filenames: false