Protocol definition parsing #1275
Conversation
Bencher Report
|
| Branch | parsing |
| Testbed | Linux |
Click to view all benchmark results
| Benchmark | hugr_bytes | Benchmark Result bytes x 1e3 (Result Δ%) | Upper Boundary bytes x 1e3 (Limit %) | hugr_nodes | Benchmark Result nodes (Result Δ%) | Upper Boundary nodes (Limit %) |
|---|---|---|---|---|---|---|
| tests/benchmarks/test_big_array.py::test_big_array_compile | 📈 view plot 🚷 view threshold | 129.06 x 1e3(0.00%)Baseline: 129.06 x 1e3 | 130.36 x 1e3 (99.01%) | 📈 view plot 🚷 view threshold | 4,873.00(0.00%)Baseline: 4,873.00 | 4,921.73 (99.01%) |
| tests/benchmarks/test_ctrl_flow.py::test_many_ctrl_flow_compile | 📈 view plot 🚷 view threshold | 21.99 x 1e3(0.00%)Baseline: 21.99 x 1e3 | 22.21 x 1e3 (99.01%) | 📈 view plot 🚷 view threshold | 693.00(0.00%)Baseline: 693.00 | 699.93 (99.01%) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## protocol-main #1275 +/- ##
=================================================
- Coverage 93.50% 93.39% -0.12%
=================================================
Files 121 123 +2
Lines 11168 11279 +111
=================================================
+ Hits 10443 10534 +91
- Misses 725 745 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # Specifying `Protocol` is redundant but we allow it optionally. | ||
| case [base] if isinstance(base, ast.Name) and base.id == "Protocol": |
There was a problem hiding this comment.
Python also allows specifying legacy generic parameters via a Protocol[T] base: https://typing.python.org/en/latest/spec/protocol.html#generic-protocols
Do we also want this?
| frame = DEF_STORE.frames[self.id] | ||
| cls_def = parse_py_class(self.python_class, frame, sources) | ||
| if cls_def.keywords: | ||
| raise GuppyError(UnexpectedError(cls_def.keywords[0], "keyword")) |
There was a problem hiding this comment.
Could you add a test that triggers this error?
There was a problem hiding this comment.
Same for the other errors in this file without coverage
| if not isinstance(v, GuppyDefinition): | ||
| raise GuppyError(NonGuppyMethodError(node, self.name, name)) |
There was a problem hiding this comment.
Do we actually want to require @guppy.declare annotations on protocol methods? We could also leave them unannotated and parse them here?
| # Prior to Python 3.13, the `__firstlineno__` attribute on classes is not set. | ||
| # However, we need this information to precisely look up the source for the | ||
| # class later. If it's not there, we can set it from the calling frame: | ||
| if not hasattr(cls, "__firstlineno__"): | ||
| cls.__firstlineno__ = frame.f_lineno # type: ignore[attr-defined] |
There was a problem hiding this comment.
This is obscure enough that it might be worth putting into a helper function now that we have this twice
| 3 | @guppy.protocol | ||
| 4 | class MyProto: | ||
| 5 | id: int | ||
| | ^^^^^^^ fields are not supported in a protocol definition |
There was a problem hiding this comment.
Nit: should be capitalised to be consistent with other errors
| 6 | class MyProto(Protocol): | ||
| | ^^^^^^^^ Protocol base is not supported | ||
|
|
||
| Help: Add a `@guppy.protocol` annotation to turn this struct into a protocol |
Closes #1209 (to be merged into
protocol-mainas the clean protocols work in progress branch)