-
Notifications
You must be signed in to change notification settings - Fork 16
[Type] ndarray typing 1: Add eval_str=True to inspect.signature() calls #411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
59fb788
[Fix] Add eval_str=True to inspect.signature() calls for stringified …
hughperkins 544bbc9
[Fix] Add eval_str=True to perf_dispatch register decorator
hughperkins 77fccda
[Fix] Add consistent error handling at all eval_str=True call sites
hughperkins 849fe2d
[Refactor] Extract get_func_signature helper for eval_str=True calls
hughperkins 46db1c7
[Test] Add test for kernels with from __future__ import annotations
hughperkins 9160366
[Lint] Move inspect/get_func_signature imports to top-level
hughperkins 5ec31d9
Merge remote-tracking branch 'origin/main' into hp/typing-t4-1-eval-str
hughperkins 84e64a4
Merge remote-tracking branch 'origin/main' into hp/typing-t4-1-eval-str
hughperkins 5f79631
[Refactor] Move get_func_signature to dedicated _signature module
hughperkins 7493617
[Lint] Sort imports after _signature module addition
hughperkins 24f1558
[Fix] Catch SyntaxError and TypeError in get_func_signature
hughperkins faeda40
[Fix] Stop catching TypeError in get_func_signature
hughperkins 0ecbd9d
Merge branch 'main' into hp/typing-t4-1-eval-str
hughperkins 2a22ba0
Merge branch 'main' into hp/typing-t4-1-eval-str
hughperkins File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import inspect | ||
| from typing import Callable | ||
|
|
||
| from quadrants.lang.exception import QuadrantsSyntaxError | ||
|
|
||
|
|
||
| def get_func_signature(func: Callable) -> inspect.Signature: | ||
| """Call ``inspect.signature`` with ``eval_str=True``. | ||
|
|
||
| ``eval_str=True`` resolves stringified annotations (PEP 563 / | ||
| ``from __future__ import annotations``) to real type objects so downstream | ||
| code can introspect them (e.g. ``dataclasses.is_dataclass``). | ||
|
|
||
| Annotation-evaluation failures (``NameError`` / ``AttributeError`` for | ||
| unresolved references, ``SyntaxError`` for malformed string annotations | ||
| such as ``"NDArray["``) are re-raised as :class:`QuadrantsSyntaxError` | ||
| with the offending function's qualified name, so users get a | ||
| Quadrants-flavored error rather than a raw ``inspect`` traceback. | ||
|
|
||
| Note: ``TypeError`` is intentionally not caught here, since | ||
| ``inspect.signature`` itself raises ``TypeError`` for non-introspectable | ||
| objects -- wrapping that as "invalid type annotation" would be | ||
| misleading. | ||
| """ | ||
| try: | ||
| return inspect.signature(func, eval_str=True) | ||
| except (NameError, AttributeError, SyntaxError) as e: | ||
| qualname = getattr(func, "__qualname__", repr(func)) | ||
| raise QuadrantsSyntaxError(f"Invalid type annotation in `{qualname}`: {e}") from e | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| """Test that kernels work with `from __future__ import annotations` (PEP 563).""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import quadrants as qd | ||
|
|
||
| from tests import test_utils | ||
|
|
||
|
|
||
| @qd.kernel | ||
| def add_kernel(a: qd.types.NDArray[qd.i32, 1], b: qd.types.NDArray[qd.i32, 1]) -> None: | ||
| for i in a: | ||
| a[i] = a[i] + b[i] | ||
|
|
||
|
|
||
| @test_utils.test() | ||
| def test_future_annotations_kernel(): | ||
| a = qd.ndarray(qd.i32, (4,)) | ||
| b = qd.ndarray(qd.i32, (4,)) | ||
| for i in range(4): | ||
| a[i] = i | ||
| b[i] = 10 | ||
| add_kernel(a, b) | ||
| for i in range(4): | ||
| assert a[i] == i + 10 |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 The
exceptclause inget_func_signatureonly catches(NameError, AttributeError), butinspect.signature(func, eval_str=True)can also raiseSyntaxErrorfor syntactically invalid string annotations (e.g.a: 'NDArray['). Such errors propagate as raw Python tracebacks instead ofQuadrantsSyntaxError, violating the docstring's promise to convert annotation-evaluation failures into user-friendly errors. Fix by addingSyntaxError(and optionallyTypeError) to theexcepttuple.Extended reasoning...
What the bug is and how it manifests
The new
get_func_signaturehelper inpython/quadrants/lang/_signature.pywrapsinspect.signature(func, eval_str=True)to resolve stringified annotations. Its docstring explicitly promises: "Annotation-evaluation failures (NameError,AttributeError) are re-raised asQuadrantsSyntaxError". However, theexceptclause only covers those two exception types, leavingSyntaxError(and others likeTypeError) unhandled.The specific code path that triggers it
At
_signature.pylines 19–23`:When a user writes a kernel with a syntactically broken string annotation (e.g.
a: 'NDArray['), Python's annotation evaluation callseval()on the string, which raisesSyntaxError: '[' was never closed. This exception is not in theexcepttuple and propagates directly to the caller as a raw Python exception.Why existing code doesn't prevent it
There is no broader
try/exceptin the callers (_func_base.py,_kernel_impl_dataclass.py,_perf_dispatch.py) that would intercept this before it reaches the user. The entire point of the helper was to be that single chokepoint for clean error translation — but it has a gap.What the impact would be
Any user who writes a kernel or
@perf_dispatchfunction with a syntactically invalid string annotation will receive a rawSyntaxErrortraceback originating from deep insideinspectrather than aQuadrantsSyntaxErrorwith a readable message. This is a documented contract violation. The PR author themselves acknowledged this as a known weakness in the "Bad / weak points" section of the PR description.How to fix it
Add
SyntaxError(and optionallyTypeError) to theexcepttuple:Step-by-step proof
FuncBase.__init__callsself.check_parameter_annotations().check_parameter_annotationscallsget_func_signature(self.func)at_signature.py:19.inspect.signature(bad_kernel, eval_str=True)internally callseval("NDArray["), which raisesSyntaxError: '[' was never closed.except (NameError, AttributeError)clause does NOT matchSyntaxError.SyntaxErrorpropagates unhandled up the call stack to the user, showing a raw traceback from insideinspectinstead of aQuadrantsSyntaxError.Empirically verified:
python3 -c "def f(a: 'NDArray['): pass; import inspect; inspect.signature(f, eval_str=True)"raisesSyntaxError, confirming the gap.