Drop Python 3.9 support#150
Merged
Merged
Conversation
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
We should be using the `X | Y` notation instead of `Union` or `Optional` Signed-off-by: Grant Ramsay <seapagan@gmail.com>
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR comprehensively modernises Python type annotations across the codebase from ChangesPython 3.10+ Type Annotation Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 16 |
| Duplication | 2 |
🟢 Coverage 100.00% diff coverage · +0.00% coverage variation
Metric Results Coverage variation ✅ +0.00% coverage variation (-1.00%) Diff coverage ✅ 100.00% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (4899405) 6517 6517 100.00% Head commit (0550248) 6491 (-26) 6491 (-26) 100.00% (+0.00%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#150) 119 119 100.00% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_context_manager.py (1)
117-141:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnsure
db_mockis closed on failure paths too.
db_mock.close()currently executes only after successful assertions. If an assertion fails earlier, this test can still leave an open connection and reintroduce warning noise.Suggested patch
- db_mock = SqliterDB(db_filename=str(db_file), auto_commit=True) - db_mock.create_table(ExampleModel) - - # Use the context manager to simulate a transaction - with db_mock: - db_mock.insert( - ExampleModel(slug="test", name="Test", content="Test content") - ) - - # After the transaction, open a new connection to query the database - new_conn = sqlite3.connect(str(db_file)) - try: - result = new_conn.execute( - "SELECT * FROM test_table WHERE slug = 'test'" - ).fetchone() - finally: - new_conn.close() - - # Assert that the data was committed - assert result is not None, "Data was not committed." - assert result[3] == "test", ( - f"Expected slug to be 'test', but got {result[3]}" - ) - - db_mock.close() + db_mock = SqliterDB(db_filename=str(db_file), auto_commit=True) + try: + db_mock.create_table(ExampleModel) + + # Use the context manager to simulate a transaction + with db_mock: + db_mock.insert( + ExampleModel( + slug="test", + name="Test", + content="Test content", + ) + ) + + # After the transaction, open a new connection to query the database + new_conn = sqlite3.connect(str(db_file)) + try: + result = new_conn.execute( + "SELECT * FROM test_table WHERE slug = 'test'" + ).fetchone() + finally: + new_conn.close() + + # Assert that the data was committed + assert result is not None, "Data was not committed." + assert result[3] == "test", ( + f"Expected slug to be 'test', but got {result[3]}" + ) + finally: + db_mock.close()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_context_manager.py` around lines 117 - 141, The test opens a SqliterDB instance (db_mock = SqliterDB(...)) and only calls db_mock.close() after assertions, so failures can leave the DB open; wrap the interaction and assertions in a try/finally (or use a pytest fixture/teardown) that always calls db_mock.close(), ensuring db_mock.close() is executed on all paths (referencing SqliterDB, db_mock, create_table, insert, and close).
🧹 Nitpick comments (2)
sqliter/sqliter.py (1)
636-643: ⚡ Quick winVerify finalizer behaviour during interpreter shutdown.
The
__del__finalizer attempts to close the SQLite connection during garbage collection. While the use ofsuppress()andgetattr()with defaults provides some safety, finalizers can still encounter issues during interpreter shutdown when imported modules (likesqlite3andcontextlib) may be partially torn down. Consider whether a context manager or explicitclose()pattern would be more reliable than relying on__del__.tests/test_m2m.py (1)
469-482: ⚡ Quick winConsider passing
raw_conntoDummyConn.__init__for clarity.The
DummyConnmethods at lines 471 and 474 referenceraw_conn, which isn't defined until line 480. Whilst this works in Python (methods aren't executed until called), it's unconventional and may confuse readers. A clearer pattern would be to passraw_connas a constructor parameter.♻️ Proposed refactor
class DummyConn: + def __init__(self, conn: sqlite3.Connection) -> None: + self._conn = conn + def cursor(self) -> DummyCursor: - return DummyCursor(raw_conn.cursor()) + return DummyCursor(self._conn.cursor()) def commit(self) -> None: - raw_conn.commit() + self._conn.commit() class DummyDB(SqliterDB): def connect(self) -> sqlite3.Connection: return cast("sqlite3.Connection", dummy_conn) raw_conn = sqlite3.connect(":memory:") - dummy_conn = DummyConn() + dummy_conn = DummyConn(raw_conn) dummy_db = DummyDB(memory=True)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_m2m.py` around lines 469 - 482, The DummyConn class currently closes over raw_conn defined later which is confusing; add an __init__(self, raw_conn: sqlite3.Connection) that stores it on self (e.g., self._raw_conn) and update cursor() and commit() to use self._raw_conn, then instantiate DummyConn(raw_conn) where dummy_conn is created; no other behavior changes are needed—DummyDB.connect should still return the dummy_conn instance.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_complex_types.py`:
- Line 196: The parameter annotation for invalid_value doesn't match the actual
parametrized inputs (one case passes a str "not a list"); update the type hint
for invalid_value in the test (the function parameter named invalid_value) to
include str (e.g., list[Any] | dict[Any, Any] | set[Any] | tuple[Any, ...] |
str) or broaden to Any/object so the annotation aligns with the test cases and
removes type-checking noise.
---
Outside diff comments:
In `@tests/test_context_manager.py`:
- Around line 117-141: The test opens a SqliterDB instance (db_mock =
SqliterDB(...)) and only calls db_mock.close() after assertions, so failures can
leave the DB open; wrap the interaction and assertions in a try/finally (or use
a pytest fixture/teardown) that always calls db_mock.close(), ensuring
db_mock.close() is executed on all paths (referencing SqliterDB, db_mock,
create_table, insert, and close).
---
Nitpick comments:
In `@tests/test_m2m.py`:
- Around line 469-482: The DummyConn class currently closes over raw_conn
defined later which is confusing; add an __init__(self, raw_conn:
sqlite3.Connection) that stores it on self (e.g., self._raw_conn) and update
cursor() and commit() to use self._raw_conn, then instantiate
DummyConn(raw_conn) where dummy_conn is created; no other behavior changes are
needed—DummyDB.connect should still return the dummy_conn instance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 45f6b15d-b407-4028-b41e-3f32c8b79912
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (49)
.github/workflows/testing.yml.pre-commit-config.yamlCONTRIBUTING.mdREADME.mddemo.pydocs/guide/foreign-keys/orm.mddocs/index.mdpyproject.tomlrequirements-dev.txtsqliter/asyncio/db.pysqliter/asyncio/orm/fields.pysqliter/asyncio/orm/m2m.pysqliter/asyncio/orm/query.pysqliter/asyncio/query.pysqliter/helpers.pysqliter/model/foreign_key.pysqliter/model/model.pysqliter/orm/fields.pysqliter/orm/m2m.pysqliter/orm/model.pysqliter/orm/query.pysqliter/orm/registry.pysqliter/query/aggregates.pysqliter/query/query.pysqliter/sqliter.pysqliter/tui/demos/base.pysqliter/tui/demos/constraints.pysqliter/tui/demos/filters.pysqliter/tui/demos/models.pysqliter/tui/demos/orm.pytests/conftest.pytests/test_aggregates.pytests/test_asyncio_core.pytests/test_bulk_insert.pytests/test_bulk_update.pytests/test_complex_types.pytests/test_context_manager.pytests/test_dates.pytests/test_foreign_keys.pytests/test_foreign_keys_orm.pytests/test_m2m.pytests/test_model.pytests/test_optional_fields.pytests/test_orm_fields.pytests/test_prefetch_related.pytests/test_query.pytests/test_select_related.pytests/test_sqliter.pytests/test_unique.py
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR drops Python 3.9 support and moves the project floor to Python 3.10.
It updates package metadata, Ruff and mypy targets, the GitHub Actions test matrix, docs, and generated dependency exports. It also updates the dev tooling stack, including pytest 9 and mypy 2, and applies the Python 3.10 typing cleanups required by the newer Ruff rules.
While validating the dependency updates, pytest 9 surfaced SQLite ResourceWarning noise from tests that left connections to be finalized by GC. The test fixtures and related edge cases now close database handles deterministically, with a small finalizer backstop for abandoned sync database instances.
Summary by CodeRabbit
New Features
Documentation
Chores