Repository navigation
Bind extension methods and emit the number protocol for implicit numeric conversions - #104
Conversation
Review follow-up: pushed e305f27Where: WhyI generated the stubs from Lean
How
Result on Lean
|
… declared operators The stubs accepted calls that fail at runtime: - Lean extensions on framework types were bound to them: every class got convert_invariant from System.Object, IEnumerable got to_py_list, IDisposable dispose_safely, StreamReader get_decimal... Python.NET only binds targets involving a non-framework type, so targets in System or Microsoft namespaces are skipped too. - Extension methods marked [EditorBrowsable(EditorBrowsableState.Never)], or declared in a class marked so, keep their static form only, like in Python.NET. - An operator the class declares suppressed only its forward synthesized form: MultiPeriodField got __radd__(float) and friends while Python.NET synthesizes an operator in both directions or not at all. Declared or inherited operators now win in both directions. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
e305f27 to
0275603
Compare
|
CI was failing on master too, since QuantConnect/Lean#9914 moved b4c9903 fixes it in The Test: |
There was a problem hiding this comment.
Requesting changes. The code looks right, but this can't merge until its runtime dependencies ship, and the PR also carries an unrelated master CI fix.
Blocking
- The runtime support hasn't shipped yet. QuantConnect/pythonnet#150 and #151 are still open, and Lean still references
QuantConnect.pythonnet2.0.66. If this merged now, the stubs would advertise members that fail at runtime for every user:order_status.is_open(),fee + 1,float(margin),order.is_contingent(),symbol.requires_mapping()and the rest. The description says as much. Please keep this open until those PRs are released and Lean is on that version. - End-to-end run on the updated Lean. Pyright and the unit tests show that the stubs are consistent with themselves, not that each member exists at runtime. Once Lean is bumped, please run a Python algorithm that calls a sample of what this PR adds and post the outcome. The sample should cover: a bound enum method (
OrderStatus.FILLED.is_open()), a bound class method (order.is_contingent()), a bound interface method (IPositionGroup.is_empty()on a position group), a generic one (slice.try_get[TradeBar](symbol, None)), and the operators onOrderFeeandInitialMarginin both directions (fee + 1,1 + fee,fee < 2,float(fee),-fee). I checked the generic case against #151's branch myself, with a test of my own:target.kind[String]()andtarget.try_get[String](1, None)work through the instance, so_Slice_TryGetand_Universe_TryGetUniversematch the runtime. - Move b4c9903 into its own PR. The
IReadOnlyDictionarybase fix has nothing to do with this feature. It repairs master's CI, which has been red since bc8c62d (run of 2026-10-16). While it rides here, master stays red for as long as this PR waits on pythonnet. On its own it's a small, separately revertable fix. - The description is out of date. It doesn't cover 0275603 (the extended type's namespace is now checked as well as the declaring one,
[EditorBrowsable(Never)]extensions aren't bound, and a declared or inherited operator suppresses both directions). Its test list is also missingFrameworkTargetsAreNotBound,EditorBrowsableNeverExtensionMethodsKeepTheStaticFormOnlyandDeclaredOrInheritedOperatorsAreNotSynthesizedInEitherDirection. Please also note that it depends on Lean#9896: until that lands,OrderTicketgets the full number protocol.
What I checked
- CI is green, and
dotnet testpasses 93/93 locally. - I generated stubs from current Lean master with
masterand with this branch, then diffed the members. Both runs used the same partial dotnet/runtime checkout, which only affectsSystemstubs. Nothing on the Lean side was removed or changed beyond reordering. The PR adds about 150 bound extension methods and the dunder set on the 11 classes with an implicit numeric conversion.System.Object,IEnumerableand the other framework types get nothing, andMultiPeriodFieldonly gets the comparisons,__neg__,__float__and__int__, as expected. - I compared the rules with #150/#151:
- Framework-only targets, open-generic targets,
EditorBrowsable(Never)on the method or its class, real members winning, and the conversion operator's parameter having to be the type itself all match. - Declared operators: pythonnet skips any
op_*name the type declares or inherits, and the stubs do the same throughDeclaredOperatorson the class and its bases. - The non-generic
IndicatorBasegets no synthesized dunders in the output because it renders as atyping.Anyalias, so this PR doesn't change it.
- Framework-only targets, open-generic targets,
- Known gaps, all non-blocking:
- pythonnet also binds closed generic and array targets over Lean types, such as
List<Symbol>andSymbol[], and the stubs skip them. That leaves editors flagging calls that would work, which is the safe direction. - For an integral conversion, pythonnet returns
intfrom+ - * neg, while the stubs always sayfloat.OrderTicketis the only such type in Lean today, and Lean#9896 excludes it. - Lean's
IAlgorithmplumbing extensions (set_runtime_error,process_security_changes,get_or_add_unrequested_security...) and many internal helpers onSymbol,OrderandSubscriptionDataConfignow show up on instances. That matches the runtime, but hiding them is up to Lean (EditorBrowsable(Never)) and should ideally happen before both changes ship.
- pythonnet also binds closed generic and array targets over Lean types, such as
Affected repositories
- QuantConnect/pythonnet#150 and #151 need to be merged and released first.
- Lean needs the pythonnet version bump, and Lean#9896.
Risk
I set risk:high. The PR adds about 150 members and the number protocol to user-facing stubs, and it depends on runtime changes that haven't shipped. The end-to-end run above on a Lean with the new pythonnet is the evidence it still lacks.
|
Replies to the blocking items:
|
There was a problem hiding this comment.
Approving. Everything I blocked on last time has been addressed, and the new commit (524a153) matches the runtime.
Since my last review
- Runtime support has shipped. I confirmed that QuantConnect/pythonnet#150, #151 and #152 are on pythonnet's master, and that QuantConnect/Lean#9964 (pythonnet 2.0.67) is merged.
- End-to-end run. The algorithm in the comment above covers every sample I asked for: a bound enum, class, interface and generic method, plus the
OrderFeeandInitialMarginoperators in both directions. Each check raises on a mismatch, so the run can only reachALL CHECKS PASSEDif they all hold. mypy also goes from 32 errors with master's stubs to 0 with this branch's. That's convincing. - b4c9903 stays in this PR. I accept the reasoning: it's one condition with its own test and a one-line change in the output, and this PR's integration check needs it to go green.
- The description is up to date.
- I'm withdrawing my inline comment on
HasImplicitConversionToString. The method already exists on master, so keeping it keeps the diff smaller.
524a153: integral conversions typed as int
- I checked it against
ImplicitNumericOps<T>in pythonnet's master, and every case matches:- It picks the first
op_Implicitto a numeric type whose parameter is the type itself. - Integral conversions give exact ints for
+ - *andnegwhen the other operand is an int (or an instance of the type), and floats otherwise. /always gives a float.- Reversed comparisons are left to Python's reflection, which needs no stub overloads.
- Declared or inherited
op_*names win.
- It picks the first
- The shared
PythonType.Int,FloatandBoolinstances are safe. The only later pass that changes types,TryReplaceGenericTypes, copies them first (new PythonType(type)). dotnet test: 93/93 pass locally. CI is green.- I generated stubs from current Lean master.
OrderTicket(integral, because Lean#9896 isn't merged yet) andOrderFee(decimal) come out as described. With pyright 1.1.338,t + 1,1 + t,-tandt * tgiveint,t + 1.5andt / 2givefloat,fee + 1and2 - feegivefloat, and there are 0 errors.
Merge timing
Lean#9896 is still open. Merging before it lands doesn't advertise anything that fails at runtime: OrderTicket really does get the number protocol, and the engine helpers really are callable. But the published stubs would show those helpers (requires_mapping, process_security_changes, ...) on Symbol, Order and IAlgorithm. As the description says, merging after Lean#9896 avoids that.
Affected repositories
- Lean: QuantConnect/Lean#9896 (the
StubsIgnoreonOrderTicketandEditorBrowsable(Never)on engine-only extensions). Nothing else needs an update.
Risk
I kept risk:high. This adds about 150 members and the number protocol to the user-facing stubs, which counts as a user-facing API addition however good the evidence is. A human should merge it, ideally after Lean#9896.
|
The Lean-side attributes moved from QuantConnect/Lean#9896 to QuantConnect/Lean#9988, which only carries the |
Python.NET (QuantConnect/pythonnet#150 and QuantConnect/pythonnet#151) binds C# extension methods to the type they extend and gives types with an implicit numeric conversion the Python number protocol. The stubs describe neither, so editors flag
order_event.status.is_open()orfee + 1as errors.The change:
MethodParserrecords the extended type of static methods whose first parameter isthis.Generator.BindExtensionMethodsclones each one onto the extended class as an instance method, dropping thethisparameter. The static form stays.SystemandMicrosoftnamespaces, generic targets, targets without a stub class and names the target or its bases already declare are skipped.[EditorBrowsable(EditorBrowsableState.Never)], or declared in a class marked so, keep their static form only.ClassParserrecords the Python number a class's implicit conversion maps to,intfor integral conversions andfloatfor the others, unless the operator is marked[StubsIgnore].Generator.AddImplicitNumericOperatorsemits__add__,__sub__and__mul__withSelfandfloatoverloads, plusintoverloads returningintfor integral conversions, their reflected forms,__truediv__returningfloatin every case, comparisons,__neg__,__float__and__int__.Class.DeclaredOperators.QuantConnect/pythonnet#150 and #151 shipped in 2.0.67, and Lean is on it since QuantConnect/Lean#9964. It still depends on QuantConnect/Lean#9988, which marks
OrderTicket'simplicit operator intwithStubsIgnoreand hides engine-only extensions withEditorBrowsable: until it lands, tickets get the full number protocol andSymbol,OrderandIAlgorithmshow helpers likerequires_mappingorprocess_security_changes.It also carries b4c9903, which keeps an
IReadOnlyDictionarybase as the .NET interface instead oftyping.Dict. That repairs master's pyright check, red since QuantConnect/Lean#9914; the details are in the comment below.Known gap: Python.NET also binds extensions whose target is a framework container of a Lean type, such as
IEnumerable<Slice>, onto the matching closed generic class. Those targets render astyping.Iterable[Slice], which has no stub class to attach to, so the stubs keep only the static form. Editors flag a few calls that work, never the reverse.Tests:
GeneratorTests.ExtensionMethodsAreBoundToTheExtendedType: enum, interface, name collision, generic and native targets, static form kept.GeneratorTests.FrameworkExtensionMethodsAreNotBoundandFrameworkTargetsAreNotBound: extensions declared in a framework namespace or targetingobject,IEnumerableand the like are not bound, their static forms stay.GeneratorTests.EditorBrowsableNeverExtensionMethodsKeepTheStaticFormOnly: hidden methods and hidden classes stay static.GeneratorTests.ImplicitNumericConversionAddsTheNumberProtocol: full dunder set for a decimal and an integral conversion, overload order, conversions from other types or markedStubsIgnoreadd nothing.GeneratorTests.DeclaredOrInheritedOperatorsAreNotSynthesizedInEitherDirection: an operator declared in one direction, or inherited, suppresses both synthesized directions.GeneratorTests.ReadOnlyDictionaryBaseIsNotNormalizedToDict: the base staysIReadOnlyDictionary, a property of that type still renders astyping.Dict.RendererTestscaseRendersBoundExtensionMethodsAndNumericOperators: rendered output of a bound enum method and the operators.OrderStatusgainsis_open,OrderFeethe number protocol,convert_invariantstays static. Lean's syntax check is unchanged at 465/470 locally.OrderFeeandInitialMarginoperators in both directions. Every call returns the expected value, and mypy accepts the algorithm with these stubs (see the comment below).