Conversation
tekknolagi
left a comment
tekknolagi
left a comment
There was a problem hiding this comment.
Thanks for the patch! I left my comments. After they're addressed, the type checking commit is good to go.
I would rather not include #1 (and its associated commit).
|
|
||
| from typing import Any, Callable, Dict, Iterable, Optional, Set, Tuple | ||
|
|
||
|
|
There was a problem hiding this comment.
Let's do Row = Dict[str, Any] and RowPredicate = Callable[[Row], bool] to make types easier to read
There was a problem hiding this comment.
Sorry which line are you referencing/
There was a problem hiding this comment.
Adding new type aliases so we can use those throughout the new type annotations. This will make nearly every annotation much shorter
| result = db.OFFSET(result, offset) | ||
| if limit: | ||
| result = db.LIMIT(result, limit) | ||
| if distinct: |
There was a problem hiding this comment.
This should go in a separate commit and needs separate tests. Additionally, in the SQL order of execution, I think it goes between select and order by
There was a problem hiding this comment.
(test is slightly less important but it should go in a separate commit than type checking for sure)
| set_values: Dict[str, Any], | ||
| pred: Callable[[Dict[str, Any]], bool] = lambda _: True, | ||
| ) -> Table: | ||
| return Table( | ||
| table.name, [{**row, **set} if pred(row) else row for row in table.rows] | ||
| table.name, | ||
| [{**row, **set_values} if pred(row) else row for row in table.rows], | ||
| ) |
There was a problem hiding this comment.
s/set_values/row_updates/g maybe
…future__` to enable postponed evaluation of type hints (PEP 563). (modern type hints for forward references) This allows updating the return type hint in the `Table.filter` method from the string literal `"Table"` to the direct type `Table`, resulting in cleaner and more modern code.
2 participants
Pyrefly (cutting edge type checker) reported several problems. Resolved all with this PR
Type Checking Results
Unit Testing