Fix type annotations and improve error reporting - #457
Conversation
The dependency bumps in #428 landed from a branch that was behind main, so these ten diagnostics never appeared on that PR and now trip every branch built on top of it. reflex_xy/component.py: spell the event-trigger annotations as `Annotated[rx.EventHandler, <args spec>]` instead of the shorthand `rx.EventHandler[<args spec>]`. The two are the same object — reflex's `EventHandler.__class_getitem__` returns exactly this Annotated form and its trigger discovery reads the spec back out of `__metadata__` — but only the shorthand is a runtime-only DSL: `EventHandler` is not a generic class, so subscripting it is invalid in a type expression. All seven triggers still resolve with the same arg-spec parameter names. xy/_validate.py: sort the rendered form of unrecognized `mark_fill` keys. `value` is user input whose keys need not be mutually comparable, so `mark_fill({1: ..., "mode": ...})` raised a bare `TypeError` out of `sorted` instead of naming the unknown key. Closed grammars are specified to raise `ValueError` (spec/api/styling.md), so this makes the implementation match. xy/pyplot/_axes.py: drop the now-redundant `cast` around `np.ma.asarray`, which the newer numpy stubs already type as `MaskedArray`. Verified against the interpreter CI resolves (3.12 / numpy 2.5.1): `ty check python tests/typing_pep561_consumer.py` is clean.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughChangesXY corrections
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR corrects Reflex event-handler annotations, simplifies masked-array handling, and ensures mixed-type unknown fill keys produce a useful validation error.
Confidence Score: 5/5The PR appears safe to merge, with no concrete changed-code defect identified. The validation change preserves strict rejection while avoiding heterogeneous-key sorting failures, the removed casts have no runtime effect, and the event annotations retain the metadata shape Reflex expects.
|
| Filename | Overview |
|---|---|
| python/reflex_xy/component.py | Rewrites seven event annotations into type-checker-compatible Annotated forms while retaining their Reflex argument metadata. |
| python/xy/_validate.py | Converts unknown fill keys to strings before sorting so heterogeneous keys are rejected through the documented validation path. |
| python/xy/pyplot/_axes.py | Removes runtime-no-op typing casts around masked-array conversion with no observable behavioral change. |
| tests/test_figure.py | Adds regression coverage confirming mixed-type unknown fill keys raise ValueError without partially mutating the figure. |
Reviews (1): Last reviewed commit: "Fix ty diagnostics surfaced by the depen..." | Re-trigger Greptile
Merging this PR will not alter performance
Comparing 
Footnotes
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 4 files
Tip: cubic could auto-approve low-risk PRs like this, if it thinks it's safe to merge. Learn more
Re-trigger cubic
1 participant
Summary
This PR addresses three separate issues: correcting EventHandler type annotations to use
Annotatedfor type-checker compatibility, removing unnecessarycast()calls that mask type information, and improving error messages when user input contains non-comparable keys.Key Changes
EventHandler type annotations (
python/reflex_xy/component.py):rx.EventHandler[<args spec>](runtime-only DSL) toAnnotated[rx.EventHandler, <args spec>](valid type expression)EventHandleris not generic, so subscripting it is invalid in type expressions and rejected by type checkers__metadata__, so both forms produce the same runtime objecton_point_hover,on_point_click,on_select_end,on_view_change,on_animation_start,on_animation_end,on_hoverRemoved unnecessary casts (
python/xy/pyplot/_axes.py):cast(np.ma.MaskedArray, ...)calls in_plot_seriesfor masked array handlingnp.ma.asarray()already returns the correct type; explicit casts were redundant and obscured intentcastimportImproved error reporting (
python/xy/_validate.py):mark_fillvalidation to handle user-supplied dicts with non-comparable keyssorted(set(value) - {...})tosorted(str(key) for key in set(value) - {...})TypeErrorwhen dict contains mixed types (e.g.,{1: "x", "mode": "y"})ValueErroras per spec/api/styling.mdtests/test_figure.pyto verify the fixImplementation Details
The EventHandler change is purely about type-checker compliance. The runtime behavior is identical because
EventHandler.__class_getitem__returns theAnnotatedform, and Reflex extracts the spec from__metadata__. The comment documents why the shorthand cannot be used in type expressions.The cast removal simplifies code by trusting NumPy's type system. The error reporting fix ensures user-facing validation errors are consistent and informative, even when input violates assumptions about key comparability.
https://claude.ai/code/session_01VhynvUCNjLBHYVfmHCR2tX
Summary by CodeRabbit
Bug Fixes
Tests