From ebb04a97de765ce5f0b6d1149c992062fa25629a Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 12 Aug 2026 13:41:33 -0700 Subject: [PATCH] Fixes for Pyright, closes #833 --- .github/workflows/test.yml | 3 + Justfile | 3 +- pyproject.toml | 1 + sqlite_utils/cli.py | 17 ++-- sqlite_utils/db.py | 135 ++++++++++++++++++-------------- sqlite_utils/utils.py | 11 ++- tests/test_cli.py | 2 +- tests/test_cli_bulk.py | 1 + tests/test_cli_insert.py | 1 + tests/test_constructor.py | 3 +- tests/test_create.py | 1 + tests/test_foreign_keys.py | 2 +- tests/test_fts.py | 2 +- tests/test_list_mode.py | 4 +- tests/test_register_function.py | 6 +- tests/test_upsert.py | 2 +- 16 files changed, 109 insertions(+), 85 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 923de2e..6c720a1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -43,6 +43,9 @@ jobs: run: pytest --sqlite-autocommit - name: run mypy run: mypy sqlite_utils tests + - name: run pyright regression checks + if: matrix.os == 'ubuntu-latest' && matrix.python-version == '3.14' + run: pyright sqlite_utils tests - name: run flake8 run: flake8 - name: run ty diff --git a/Justfile b/Justfile index be41523..e93075f 100644 --- a/Justfile +++ b/Justfile @@ -8,11 +8,12 @@ @run *options: uv run -- {{options}} -# Run linters: black, flake8, mypy, ty, cog +# Run linters: black, flake8, mypy, pyright, ty, cog @lint: just run black . --check uv run flake8 uv run mypy sqlite_utils tests + uv run pyright sqlite_utils tests uv run ty check sqlite_utils uv run cog --check README.md docs/*.rst uv run --group docs codespell docs/*.rst --ignore-words docs/codespell-ignore-words.txt diff --git a/pyproject.toml b/pyproject.toml index 6bc0a64..9b4d6f5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,6 +48,7 @@ dev = [ # flake8 "flake8", "flake8-pyproject", + "pyright>=1.1.411", "ty>=0.0.37", # For stable cog: "tabulate>=0.10.0", diff --git a/sqlite_utils/cli.py b/sqlite_utils/cli.py index d9c7728..c90c137 100644 --- a/sqlite_utils/cli.py +++ b/sqlite_utils/cli.py @@ -1093,7 +1093,7 @@ def insert_upsert_implementation( column_type_overrides = {column: ctype.upper() for column, ctype in (types or [])} def _insert_docs(docs, tracker=None): - extra_kwargs = { + extra_kwargs: dict[str, Any] = { "ignore": ignore, "replace": replace, "truncate": truncate, @@ -3275,15 +3275,10 @@ def convert( raise click.ClickException(str(e)) if dry_run: # Pull first 20 values for first column and preview them - if multi: - - def preview(v): + def preview(v): + if multi: return json.dumps(fn(v), default=repr, ensure_ascii=False) if v else v - - else: - - def preview(v): - return fn(v) if v else v + return fn(v) if v else v db.conn.create_function("preview_transform", 1, preview) sql = """ @@ -3788,12 +3783,12 @@ def _rows_from_code(code): code = pathlib.Path(code).read_text() except FileNotFoundError: raise click.ClickException(f"File not found: {code}") - namespace = {} + namespace: dict[str, Any] = {} try: exec(code, namespace) # noqa: S102 except SyntaxError as ex: raise click.ClickException(f"Error in --code: {ex}") - rows = namespace.get("rows") + rows: Any = namespace.get("rows") if callable(rows): rows = rows() if isinstance(rows, dict): diff --git a/sqlite_utils/db.py b/sqlite_utils/db.py index 82a95c1..66dc700 100644 --- a/sqlite_utils/db.py +++ b/sqlite_utils/db.py @@ -18,6 +18,7 @@ from dataclasses import dataclass, field from types import TracebackType from typing import ( Any, + TypeVar, Union, cast, ) @@ -256,8 +257,8 @@ class ForeignKey: column: str | None = field(compare=False) other_table: str other_column: str | None = field(compare=False) - columns: tuple[str, ...] = () - other_columns: tuple[str, ...] = () + columns: tuple[str, ...] | list[str] = () + other_columns: tuple[str, ...] | list[str] = () is_compound: bool = False on_delete: str = "NO ACTION" on_update: str = "NO ACTION" @@ -320,6 +321,8 @@ ForeignKeyIndicator = ( ForeignKeysType = Iterable[ForeignKeyIndicator] | list[ForeignKeyIndicator] +PrimaryKey = str | tuple[str, ...] | list[str] + class Default: pass @@ -327,6 +330,8 @@ class Default: DEFAULT = Default() +T = TypeVar("T") + Tracer = Callable[[str, Sequence[Any] | dict[str, Any] | None], None] @@ -1853,8 +1858,8 @@ class Database: fk_object = self._resolve_foreign_key_casing( fk_object, table_obj.columns_dict ) - columns = fk_object.columns - other_columns = fk_object.other_columns + columns = tuple(fk_object.columns) + other_columns = tuple(fk_object.other_columns) for column in columns: if column not in table_obj.columns_dict: raise AlterError(f"No such column: {column} in {table}") @@ -1914,9 +1919,10 @@ class Database: existing_indexes = {tuple(i.columns) for i in table.indexes} for fk in table.foreign_keys: # A compound foreign key gets a single composite index - if fk.columns not in existing_indexes: + fk_columns = tuple(fk.columns) + if fk_columns not in existing_indexes: table.create_index(fk.columns, find_unique_name=True) - existing_indexes.add(fk.columns) + existing_indexes.add(fk_columns) def vacuum(self) -> None: "Run a SQLite ``VACUUM`` against the database." @@ -2453,7 +2459,7 @@ class Table(Queryable): replace: bool = False, ignore: bool = False, transform: bool = False, - strict: bool | Default = DEFAULT, + strict: bool | Default | None = DEFAULT, ) -> "Table": """ Create a table with the specified columns. @@ -2524,7 +2530,7 @@ class Table(Queryable): replace=replace, ignore=ignore, transform=transform, - strict=strict, # type: ignore[arg-type] + strict=cast(bool, strict), ) return self @@ -2860,7 +2866,7 @@ class Table(Queryable): for name, type_ in current_column_pairs: type_ = types.get(name) or type_ if name in drop: - del [copy_from_to[name]] + del copy_from_to[name] continue new_name = rename.get(name) or name new_column_pairs.append((new_name, type_)) @@ -3343,7 +3349,10 @@ class Table(Queryable): :param on_update: ``ON UPDATE`` action for the foreign key. """ columns = (column,) if isinstance(column, str) else tuple(column) + if not columns: + raise ValueError("column must contain at least one column name") columns = tuple(resolve_casing(c, self.columns_dict) for c in columns) + assert columns # Ensure columns exist for col in columns: if col not in self.columns_dict: @@ -3354,7 +3363,7 @@ class Table(Queryable): raise ValueError( "other_table must be specified for a compound foreign key" ) - other_table = self.guess_foreign_table(columns[0]) + other_table = self.guess_foreign_table(next(iter(columns))) # If other_column is not specified, detect the primary key on other_table if other_column is None: if len(columns) > 1: @@ -3801,8 +3810,10 @@ class Table(Queryable): for row in cursor: yield dict(zip(columns, row)) - def value_or_default(self, key: str, value: Any) -> Any: - return self._defaults[key] if value is DEFAULT else value + def value_or_default(self, key: str, value: T | Default) -> T: + if value is DEFAULT: + return cast(T, self._defaults[key]) + return cast(T, value) def delete(self, pk_values: list | tuple | str | float) -> "Table": """ @@ -3987,7 +3998,7 @@ class Table(Queryable): def _convert_multi( self, column, fn, drop, show_progress, where=None, where_args=None - ): + ) -> "Table": # First we execute the function pk_to_values = {} new_column_types: dict[str, set[type]] = {} @@ -4033,6 +4044,7 @@ class Table(Queryable): bar.update(1) if drop: self.transform(drop=(column,)) + return self def build_insert_queries_and_params( self, @@ -4336,8 +4348,8 @@ class Table(Queryable): def insert( self, record: dict[str, Any], - pk=DEFAULT, - foreign_keys=DEFAULT, + pk: PrimaryKey | Default | None = DEFAULT, + foreign_keys: ForeignKeysType | Default | None = DEFAULT, column_order: list[str] | Default | None = DEFAULT, not_null: Iterable[str] | Default | None = DEFAULT, defaults: dict[str, Any] | Default | None = DEFAULT, @@ -4405,24 +4417,24 @@ class Table(Queryable): def insert_all( self, records: Iterable[dict[str, Any]] | Iterable[Sequence[Any]], - pk=DEFAULT, - foreign_keys=DEFAULT, - column_order=DEFAULT, - not_null=DEFAULT, - defaults=DEFAULT, - batch_size=DEFAULT, - hash_id=DEFAULT, - hash_id_columns=DEFAULT, - alter=DEFAULT, - ignore=DEFAULT, - replace=DEFAULT, - truncate=False, - extracts=DEFAULT, - conversions=DEFAULT, - columns=DEFAULT, - upsert=False, - analyze=False, - strict=DEFAULT, + pk: PrimaryKey | Default | None = DEFAULT, + foreign_keys: ForeignKeysType | Default | None = DEFAULT, + column_order: list[str] | Default | None = DEFAULT, + not_null: Iterable[str] | Default | None = DEFAULT, + defaults: dict[str, Any] | Default | None = DEFAULT, + batch_size: int | Default = DEFAULT, + hash_id: str | Default | None = DEFAULT, + hash_id_columns: Iterable[str] | Default | None = DEFAULT, + alter: bool | Default | None = DEFAULT, + ignore: bool | Default | None = DEFAULT, + replace: bool | Default | None = DEFAULT, + truncate: bool = False, + extracts: dict[str, str] | list[str] | Default | None = DEFAULT, + conversions: dict[str, str] | Default | None = DEFAULT, + columns: dict[str, Any] | Default | None = DEFAULT, + upsert: bool = False, + analyze: bool = False, + strict: bool | Default | None = DEFAULT, ) -> "Table": """ Like ``.insert()`` but takes a list of records and ensures that the table @@ -4715,6 +4727,7 @@ class Table(Queryable): elif isinstance(pk, str): self.last_pk = row[resolve_casing(pk, row)] else: + assert pk is not None self.last_pk = tuple( row[resolve_casing(p, row)] for p in pk ) @@ -4732,6 +4745,7 @@ class Table(Queryable): pk_index = column_names.index(resolve_casing(pk, column_names)) self.last_pk = first_record_list[pk_index] else: + assert pk is not None self.last_pk = tuple( first_record_list[ column_names.index(resolve_casing(p, column_names)) @@ -4743,6 +4757,7 @@ class Table(Queryable): if hash_id: self.last_pk = hash_record(first_record_dict, hash_id_columns) else: + assert pk is not None self.last_pk = ( first_record_dict[resolve_casing(pk, first_record_dict)] if isinstance(pk, str) @@ -4759,19 +4774,19 @@ class Table(Queryable): def upsert( self, - record, - pk=DEFAULT, - foreign_keys=DEFAULT, - column_order=DEFAULT, - not_null=DEFAULT, - defaults=DEFAULT, - hash_id=DEFAULT, - hash_id_columns=DEFAULT, - alter=DEFAULT, - extracts=DEFAULT, - conversions=DEFAULT, - columns=DEFAULT, - strict=DEFAULT, + record: dict[str, Any], + pk: PrimaryKey | Default | None = DEFAULT, + foreign_keys: ForeignKeysType | Default | None = DEFAULT, + column_order: list[str] | Default | None = DEFAULT, + not_null: Iterable[str] | Default | None = DEFAULT, + defaults: dict[str, Any] | Default | None = DEFAULT, + hash_id: str | Default | None = DEFAULT, + hash_id_columns: Iterable[str] | Default | None = DEFAULT, + alter: bool | Default | None = DEFAULT, + extracts: dict[str, str] | list[str] | Default | None = DEFAULT, + conversions: dict[str, str] | Default | None = DEFAULT, + columns: dict[str, Any] | Default | None = DEFAULT, + strict: bool | Default | None = DEFAULT, ) -> "Table": """ Like ``.insert()`` but performs an ``UPSERT``, where records are inserted if they do @@ -4798,20 +4813,20 @@ class Table(Queryable): def upsert_all( self, records: Iterable[dict[str, Any]] | Iterable[Sequence[Any]], - pk=DEFAULT, - foreign_keys=DEFAULT, - column_order=DEFAULT, - not_null=DEFAULT, - defaults=DEFAULT, - batch_size=DEFAULT, - hash_id=DEFAULT, - hash_id_columns=DEFAULT, - alter=DEFAULT, - extracts=DEFAULT, - conversions=DEFAULT, - columns=DEFAULT, - analyze=False, - strict=DEFAULT, + pk: PrimaryKey | Default | None = DEFAULT, + foreign_keys: ForeignKeysType | Default | None = DEFAULT, + column_order: list[str] | Default | None = DEFAULT, + not_null: Iterable[str] | Default | None = DEFAULT, + defaults: dict[str, Any] | Default | None = DEFAULT, + batch_size: int | Default = DEFAULT, + hash_id: str | Default | None = DEFAULT, + hash_id_columns: Iterable[str] | Default | None = DEFAULT, + alter: bool | Default | None = DEFAULT, + extracts: dict[str, str] | list[str] | Default | None = DEFAULT, + conversions: dict[str, str] | Default | None = DEFAULT, + columns: dict[str, Any] | Default | None = DEFAULT, + analyze: bool = False, + strict: bool | Default | None = DEFAULT, ) -> "Table": """ Like ``.upsert()`` but can be applied to a list of records. diff --git a/sqlite_utils/utils.py b/sqlite_utils/utils.py index ed5a558..3145a6f 100644 --- a/sqlite_utils/utils.py +++ b/sqlite_utils/utils.py @@ -14,6 +14,7 @@ from typing import ( TYPE_CHECKING, Any, BinaryIO, + Generic, TypeVar, Union, cast, @@ -344,7 +345,11 @@ def rows_from_file( reader = csv.DictReader(decoded_fp, dialect=dialect) else: reader = csv.DictReader(decoded_fp) - rows = _extra_key_strategy(reader, ignore_extras, extras_key) + rows = _extra_key_strategy( + cast(Iterable[dict[str | None, object]], reader), + ignore_extras, + extras_key, + ) return _CloseableIterator(iter(rows), decoded_fp), Format.CSV elif format == Format.TSV: rows, _ = rows_from_file( @@ -487,12 +492,12 @@ class ValueTracker: del self.couldbe[key] -class NullProgressBar: +class NullProgressBar(Generic[T]): def __init__(self, *args: Iterable[T]) -> None: self.args = args def __iter__(self) -> Iterator[T]: - yield from self.args[0] # type: ignore + yield from self.args[0] def update(self, value: int) -> None: pass diff --git a/tests/test_cli.py b/tests/test_cli.py index 012900c..f60c7d5 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1220,7 +1220,7 @@ def test_rows(db_path, args, expected): {"id": 1, "age": 4, "name": "Cleo"}, {"id": 2, "age": 2, "name": "Pancakes"}, ], - column_order=("id", "name", "age"), + column_order=["id", "name", "age"], ) result = CliRunner().invoke(cli.cli, ["rows", db_path, "dogs"] + args) assert expected == result.output.strip() diff --git a/tests/test_cli_bulk.py b/tests/test_cli_bulk.py index 24889b3..c5c9dcd 100644 --- a/tests/test_cli_bulk.py +++ b/tests/test_cli_bulk.py @@ -91,6 +91,7 @@ def test_cli_bulk_batch_size(test_db_and_path): stdin=subprocess.PIPE, stdout=sys.stdout, ) + assert proc.stdin is not None # Writing one record should not commit proc.stdin.write(b'{"id": 3, "name": "Three"}\n\n') proc.stdin.flush() diff --git a/tests/test_cli_insert.py b/tests/test_cli_insert.py index 01e7e94..0117862 100644 --- a/tests/test_cli_insert.py +++ b/tests/test_cli_insert.py @@ -577,6 +577,7 @@ def test_insert_streaming_batch_size_1(db_path): stdin=subprocess.PIPE, stdout=sys.stdout, ) + assert proc.stdin is not None proc.stdin.write(b'{"name": "Azi"}\n') proc.stdin.flush() diff --git a/tests/test_constructor.py b/tests/test_constructor.py index 2d0a298..412b66c 100644 --- a/tests/test_constructor.py +++ b/tests/test_constructor.py @@ -83,7 +83,8 @@ def test_autocommit_connections_are_rejected(tmpdir, autocommit): ) def test_legacy_transaction_control_connection_is_accepted(tmpdir): conn = sqlite3.connect( - str(tmpdir / "test.db"), autocommit=sqlite3.LEGACY_TRANSACTION_CONTROL + str(tmpdir / "test.db"), + autocommit=sqlite3.LEGACY_TRANSACTION_CONTROL, # type: ignore[arg-type] ) db = Database(conn) db.table("t").insert({"id": 1}, pk="id") diff --git a/tests/test_create.py b/tests/test_create.py index 0af68a6..83ce403 100644 --- a/tests/test_create.py +++ b/tests/test_create.py @@ -1182,6 +1182,7 @@ def test_works_with_pathlib_path(tmpdir): @pytest.mark.skipif(pd is None, reason="pandas and numpy are not installed") def test_create_table_numpy(fresh_db): + assert pd is not None df = pd.DataFrame({"col 1": range(3), "col 2": range(3)}) fresh_db.table("pandas").insert_all(df.to_dict(orient="records")) assert [ diff --git a/tests/test_foreign_keys.py b/tests/test_foreign_keys.py index 271125c..f916c65 100644 --- a/tests/test_foreign_keys.py +++ b/tests/test_foreign_keys.py @@ -608,7 +608,7 @@ def test_foreign_key_is_immutable(): fk = ForeignKey("c", "pid", "p", "id") with pytest.raises(dataclasses.FrozenInstanceError): - fk.table = "other" + setattr(fk, "table", "other") def test_foreign_key_equality_and_hash_include_actions(): diff --git a/tests/test_fts.py b/tests/test_fts.py index 312b032..04b5bc3 100644 --- a/tests/test_fts.py +++ b/tests/test_fts.py @@ -510,7 +510,7 @@ def test_view_has_no_enable_fts(): db.create_view("hello", "select 1 + 1") # Views deliberately do not have an enable_fts() method with pytest.raises(AttributeError): - db.view("hello").enable_fts() # type: ignore[union-attr] + db.view("hello").enable_fts() # type: ignore[attr-defined] @pytest.mark.parametrize( diff --git a/tests/test_list_mode.py b/tests/test_list_mode.py index b9ab812..75f5a76 100644 --- a/tests/test_list_mode.py +++ b/tests/test_list_mode.py @@ -99,7 +99,7 @@ def test_list_mode_error_non_string_columns(): yield ["a", "b", "c"] with pytest.raises(ValueError, match="must be a list of column name strings"): - db.table("bad").insert_all(bad_data()) + db.table("bad").insert_all(bad_data()) # type: ignore[arg-type] def test_list_mode_error_mixed_types(): @@ -111,7 +111,7 @@ def test_list_mode_error_mixed_types(): yield {"id": 1, "name": "Alice"} # Should be a list, not dict with pytest.raises(ValueError, match="must also be lists"): - db.table("bad").insert_all(bad_data()) + db.table("bad").insert_all(bad_data()) # type: ignore[arg-type] def test_list_mode_empty_after_headers(): diff --git a/tests/test_register_function.py b/tests/test_register_function.py index 618bf1e..63f0570 100644 --- a/tests/test_register_function.py +++ b/tests/test_register_function.py @@ -86,21 +86,21 @@ def test_register_function_deterministic_tries_again_if_exception_raised(fresh_d def test_register_function_replace(fresh_db): @fresh_db.register_function() - def one(): + def one(): # pyright: ignore[reportRedeclaration] return "one" assert "one" == fresh_db.execute("select one()").fetchone()[0] # This will silently fail to replaec the function @fresh_db.register_function() - def one(): # noqa + def one(): # pyright: ignore[reportRedeclaration] return "two" assert "one" == fresh_db.execute("select one()").fetchone()[0] # This will replace it @fresh_db.register_function(replace=True) - def one(): # noqa + def one(): # pyright: ignore[reportRedeclaration] return "two" assert "two" == fresh_db.execute("select one()").fetchone()[0] diff --git a/tests/test_upsert.py b/tests/test_upsert.py index 0f44cc7..8274557 100644 --- a/tests/test_upsert.py +++ b/tests/test_upsert.py @@ -8,7 +8,7 @@ from sqlite_utils.db import PrimaryKeyRequired def test_upsert(use_old_upsert): db = Database(memory=True, use_old_upsert=use_old_upsert) table = db.table("table") - table.insert({"id": 1, "name": "Cleo"}, pk="id") + table.insert_all([{"id": 1, "name": "Cleo"}], pk="id", replace=True) table.upsert({"id": 1, "age": 5}, pk="id", alter=True) assert list(table.rows) == [{"id": 1, "name": "Cleo", "age": 5}] assert table.last_pk == 1