Type Annotations for RestrictedPython#317
Conversation
…mer to start looking into it
# Conflicts: # .meta.toml # CHANGES.rst # setup.py # pyproject.toml # src/RestrictedPython/transformer.py
# Conflicts: # .meta.toml
changing the return type in compile_restricted_function to match the rest of the functions
icemac
left a comment
icemac
left a comment
There was a problem hiding this comment.
I like the idea of adding type annotations but I am missing checks that the type annotations are correct (mypy or ty preferably run via pre-commit.) Otherwise we cannot say that the current type annotations are correct and will stay correct with future changes.
There was a problem hiding this comment.
Pull request overview
Adds first-class typing support (PEP 561) across RestrictedPython by introducing type annotations in key modules and marking the distribution as typed.
Changes:
- Add/expand type annotations in the compiler and AST transformer code paths.
- Introduce
py.typedand advertise typing support via package metadata. - Minor typing-related cleanup in utilities and changelog messaging.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/RestrictedPython/Utilities.py | Adds typing for reorder() and imports Iterable. |
| src/RestrictedPython/transformer.py | Adds extensive annotations to transformer helpers and visit_* methods. |
| src/RestrictedPython/py.typed | Adds PEP 561 marker file for typed package distribution. |
| src/RestrictedPython/compile.py | Adds annotations, CompileResult as NamedTuple, and typed signatures for compile helpers. |
| pyproject.toml | Declares Typing :: Typed and adds a typecheck extra. |
| CHANGES.rst | Adds changelog entry for type-annotation work. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| policy=RestrictingNodeTransformer): | ||
| source: str | ReadableBuffer | Module | Expression | Interactive, | ||
| filename: str | ReadableBuffer | PathLike[Any] = '<string>', | ||
| mode: Literal["exec", "eval", "single"] = "exec", |
| policy=RestrictingNodeTransformer): | ||
| source: str | ReadableBuffer | Module | Expression | Interactive, | ||
| filename: str | ReadableBuffer | PathLike[Any] = '<unknown>', | ||
| mode: str = 'exec', |
icemac
left a comment
icemac
left a comment
There was a problem hiding this comment.
I like the changes. There are still some open conversations, could you please resolve them?
| _T_pos_ast: typing.TypeAlias = ( | ||
| ast.stmt | ast.expr | ast.excepthandler | ast.arg | ast.keyword | ast.alias | ||
| | ast.pattern) | ||
| if sys.version_info >= (3, 12): |
There was a problem hiding this comment.
Please use _compat.IS_PY312_OR_GREATER instead
There was a problem hiding this comment.
mypy and other static type checkers have limited if/else support. "if sys.version_info >= (...)" they support, but "if some_bool_value" does not. I suggest allowing "if sys.version_info >= (...)" for types, but only in _types.py.
I will resolve them after #318 is resolved. |
This PR is based on #303. But PR #303 is not active.
Closes #303