Repository navigation
Show an overridden ToString() in the default repr of .NET objects - #156
Conversation
Containers such as lists print their elements with repr(), so a list of .NET objects whose type overrides ToString() printed only opaque '<Type object at 0x...>' entries. Like IronPython, the default repr now appends the ToString() text: '<QuantConnect.Symbol object at 0x... [SPY]>'. Types without a ToString() override, or whose ToString() returns null or throws, keep the previous repr, and a public __repr__() still wins. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Martin-Molinero
left a comment
There was a problem hiding this comment.
[<QuantConnect.Symbol object at 0x714c42b85f00 [SPY]>, <QuantConnect.Symbol object at 0x714c42b85fc0 [AIG]>]
No, let's remove this please QuantConnect.Symbol object at 0x714c42b85f00 end users not interested on it, have no gain/info from it
Drop the '<Type object at 0x...>' wrapper: end users get nothing from the type name and address, so a list of Symbols now prints as [SPY, AIG]. Types without a ToString() override, or whose ToString() returns null or throws, keep the default object repr, and a public __repr__() still wins. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Removed the These cases are unchanged:
Tests are updated to assert the exact text, and
|
There was a problem hiding this comment.
Requesting changes. The approach does what #69 asks and what was requested in the earlier review: a list of Symbols prints as [SPY, AIG]. A public __repr__() still wins, types without a ToString() override keep the default repr, and a ToString() that throws or returns null falls back to that default. CI is green, including the Lean Python unit and regression checks.
I built the branch and probed repr() on types the new tests don't cover. Two of them come out wrong:
-
Enums:
repr()disagrees withstr().EnumoverridesToString(), so enums now take the new branch and return the .NET name, butEnumObject.tp_strreturns the Python-facing snake-case name:repr(DayOfWeek.Monday) -> Monday str(DayOfWeek.Monday) -> MONDAY [DayOfWeek.Monday, FileAccess.ReadWrite] -> [Monday, ReadWrite]A list of enum values, such as resolutions, would print names that don't match the ones users type and that
str()returns. UseEnumObject.ToPythonString(e)forEnuminstances sorepr()andstr()agree, and add a test for it intests/test_repr.pyortests/test_enum.py. -
Structs without their own
ToString()now print a bare CLR type name.System.ValueTypeoverridesToString()to return the type's full name, so theDeclaringType != typeof(object)check passes for every struct:repr(List[int]().GetEnumerator()) -> System.Collections.Generic.List`1+Enumerator[System.Int32]Before this change it printed
<System.Collections.Generic.List[Int32]... object at 0x...>. That's less useful than before, and it looks like a type rather than an instance. Also excludetypeof(ValueType)as the declaring type so these keep the default repr, and add a test.
Also:
- CHANGELOG: the recent changes (#150, #151, #154) each added an entry under
[Unreleased], and this one doesn't. Please add one that saysrepr()now uses an overriddenToString().
What I checked and found fine: exceptions keep their own tp_repr (Exception('boom')). TimeSpan still converts to datetime.timedelta. A Python subclass that overrides ToString in Python gets that text. A Python ToString that raises is caught and falls back. The extra GetMethod reflection lookup is the same kind of lookup tp_repr already does for __repr__. The scope matches the issue.
Affected repositories: Lean picks this up only through a QuantConnect.pythonnet package version bump. Lean's Python tests pass with this build, per the CI checks above.
Risk label: risk:high. This changes the repr() text of every .NET object whose type overrides ToString(). Users see that text, and user code that parses or compares it can break. That makes it a user-facing behavior change, which isn't risk:low whatever the evidence.
| } | ||
|
|
||
| // show an overridden ToString() so containers of .NET objects print readably: [SPY, AIG] | ||
| if (instType.GetMethod("ToString", Type.EmptyTypes)?.DeclaringType != typeof(object)) |
There was a problem hiding this comment.
This also matches Enum (its ToString() gives Monday while str() gives MONDAY) and every struct without its own override, because System.ValueType.ToString() returns the type's full name, e.g. System.Collections.Generic.List1+Enumerator[System.Int32]. Please use EnumObject.ToPythonStringfor enums and excludetypeof(ValueType)` here, so those structs keep the default repr.
Enum.ToString() gives the .NET member name, while str() of an enum gives the Python-facing one (MONDAY), so repr() now uses the same name. ValueType.ToString() only returns the type name, so structs without their own ToString() keep the default object repr. Add a CHANGELOG entry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed the review:
Test results:
|
There was a problem hiding this comment.
Requesting changes, for one remaining case of the problem the last review raised for structs.
Since my last review: both requests and the CHANGELOG entry are done. Enums use EnumObject.ToPythonString, so [DayOfWeek.Monday, FileAccess.ReadWrite] prints [MONDAY, READ_WRITE]. Structs whose ToString() comes from ValueType keep the default repr. Both have tests. CI is green on the head commit 657faff, including the Lean Python unit and regression checks. The branch merges cleanly with current master (#157).
Python subclasses with __namespace__/__assembly__ that don't define ToString now print a bare type name. For these classes, ClassDerived generates a CLR type that overrides every virtual method, including Object.ToString(). So the DeclaringType check always passes. When the Python class doesn't define ToString, the generated override calls object.ToString(), which returns the type's full name. I built the branch and ran:
class PlainNs(System.Object):
__namespace__ = "rvtest2"
repr(PlainNs()) # -> rvtest2.PlainNs (before: <...PlainNs object at 0x...>)The same subclass without __namespace__ keeps the default repr. A __namespace__ subclass of a type that does override ToString(), like ReprTest.Bar, correctly gets the base text. Please handle IPythonDerivedType instances so they keep the default repr unless something real overrides ToString: either the Python class defines ToString, or a CLR base below the generated type overrides it. For example, resolve the declaring type from the first non-generated base type when the Python type has no ToString attribute. Please add a test in tests/test_repr.py for both a __namespace__ subclass of System.Object and one of ReprTest.Bar. Lean doesn't use __namespace__ in its Python code, so this affects users who subclass .NET types that way, not Lean's own paths.
What I checked and found fine:
- Plain Python subclasses (no
__namespace__) ofBarshow its text, and those ofList[int]andSystem.Objectkeep the default repr. GuidandVersionshow their text inside a list.tests/test_repr.pyandtests/test_class.pypass locally on this build.- In Lean, the two places that mention the old
<QuantConnect.Symbol object at 0x...>text don't depend on it.KeyErrorPythonExceptionInterpreterreads the key viastr()first, andExtensions.TryCreateTypeonly matches a<class 'prefix.
Affected repositories: Lean picks this up through a QuantConnect.pythonnet package version bump. Its Python tests pass with this build, per the CI checks.
Risk label: risk:high (unchanged). This changes the repr() text users see for every .NET object whose type overrides ToString(), and user code that parses or compares that text can break. That user-facing behavior change keeps it out of risk:low whatever the evidence.
| // show an overridden ToString() so containers of .NET objects print readably: [SPY, AIG]. | ||
| // ValueType.ToString() only returns the type name, so structs without their own keep the default | ||
| var toStringType = instType.GetMethod("ToString", Type.EmptyTypes)?.DeclaringType; | ||
| if (toStringType != typeof(object) && toStringType != typeof(ValueType)) |
There was a problem hiding this comment.
For __namespace__/__assembly__ subclasses, toStringType is always the generated derived type, because ClassDerived overrides every virtual method. When the Python class has no ToString, that override returns object.ToString(), so repr() becomes the bare type name, e.g. rvtest2.PlainNs. Please handle IPythonDerivedType here: look through the generated type to its CLR base unless the Python class defines ToString.
The generated type of a Python subclass with __namespace__ overrides every virtual method, ToString() included, so its DeclaringType always passed the check and repr() returned the bare type name. Like PythonDerivedType.InvokeMethod, only count a ToString the Python class defines; otherwise look through the generated types to the .NET base. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed the review: How it works: for Results:
All three cases are covered by Test results:
|
There was a problem hiding this comment.
Approving. The last open request is done, and I found nothing new that blocks.
Since my last review (33cc7da): for __namespace__/__assembly__ subclasses, tp_repr now counts ToString as overridden only when the Python class defines it. It uses the same "not a managed method binding" check as PythonDerivedType.InvokeMethod (src/runtime/Types/ClassDerived.cs:722-728). Otherwise it walks past the generated types to the .NET base. test_namespace_subclass covers a subclass of System.Object, a subclass of ReprTest.Bar, and one that defines ToString in Python.
I built the head commit. tests/test_repr.py, tests/test_class.py and tests/test_subclass.py pass: 60 passed, 1 skipped. I also probed cases the new tests don't cover:
- A
__namespace__class that definesToStringin Python shows that text. So does a plain Python subclass of it. - A plain Python subclass of a
__namespace__System.Objectsubclass that definesToStringshows that text. - A Python
ToStringthat raises keeps the default repr. - A
__getattr__that raises keeps the default repr, and leaves no Python error set. [B(), D()]mixes base-type text and the default repr as expected.- A
__namespace__subclass of a__namespace__class whoseToStringis defined in Python shows the .NET base'sToString()text, not the Python one. This doesn't come from this change:str(),b.ToString()andString.Format("{0}", b)already return that same text. The derived type's managedToStringbinding shadows the Python method in the lookup, sorepr()agrees with them.
The extra attribute lookup only runs for instances of generated derived types, and only when repr() is called, so it doesn't add cost to a hot path.
Unchanged from earlier reviews and still fine:
- Enums match
str(). - Structs whose
ToString()comes fromValueTypekeep the default repr. - A public
__repr__()still takes precedence. - A
ToString()that throws or returnsnullfalls back to the default repr. - The CHANGELOG has an entry.
- The scope matches #69.
CI: all green on 33cc7da, including lean-python-unit-tests and lean-python-regression.
Affected repositories: Lean picks this up through a QuantConnect.pythonnet package version bump, which still needs to happen. Lean's Python tests pass with this build.
Risk label: risk:high (unchanged). This changes the repr() text users see for every .NET object whose type overrides ToString(), and user code that parses or compares that text can break. Because it's a user-facing behavior change, no amount of evidence makes it risk:low, so a person should merge it.
Closes #69
What changed
Python containers print their elements with
repr(), and the defaultrepr()of a .NET object ignoredToString(). So a list of objects whose type overridesToString(), such asSymbol, printed only opaque entries:ClassBase.tp_reprnow returns theToString()text when the runtime type overridesToString():__repr__()on the type still takes precedence, as before.str():[DayOfWeek.Monday, FileAccess.ReadWrite]prints[MONDAY, READ_WRITE].ToString()keep the exact previous<Type object at 0x...>repr. That covers types where it is declared onSystem.Object, and structs where it is declared onSystem.ValueType, which only returns the type name.__namespace__get a generated .NET type that overrides every virtual method,ToString()included. For these, only aToStringthe Python class defines, or one a .NET base type overrides, counts; otherwise the default repr is kept. This is the same checkPythonDerivedType.InvokeMethoduses to tell a Python override from the inherited managed method.ToString()returnsnullor throws,repr()falls back to the previous default instead of failing, sorepr()doesn't raise anywhere it used to succeed.str()is unchanged.Tests
tests/test_repr.py: theToString()-only cases (Bar,BazBase,BazMiddle,Baz,System.String) now assert thatrepr()equals theToString()text. New tests cover a list of such objects,System.Object(no override, so the default repr is kept), andToString()returningnullor throwing (newReprTest.NullToString/ThrowingToStringfixtures). New tests also cover enums (repr()equalsstr(), also inside a list) and a struct without its ownToString()(List[int]().GetEnumerator()keeps the default repr).test_namespace_subclasscovers__namespace__subclasses ofSystem.Object(default repr), ofReprTest.Bar(its text) and one definingToStringin Python (that text).tests/test_class.py::test_class_default_reprnow checks the default repr onSystem.Object, sinceSystem.StringoverridesToString().pytest --runtime netcore tests: 484 passed, 23 skipped, 1 failed. The failure istest_module.py::test_explicit_assembly_load(assembly not found in this environment), and it fails the same way onmaster.dotnet test src/embed_tests/: 1028 passed, 8 skipped, 0 failed. The isolatedSetPythonPathrun: 1 passed.dotnet test src/python_tests_runner/: 1 passed, 1 failed (No module named 'tests.utils', environment path issue). It fails the same way onmaster.Python.Runtime.dll, running the same Python unit test selection as theLean Python Unit Testsworkflow: 39220 passed, 100 skipped, 0 failed.SPYandAIGand fails unlessstr(symbols) == "[SPY, AIG]". The same algorithm checksstr([Resolution.DAILY, Resolution.MINUTE]) == "[DAILY, MINUTE]". It logged[SPY, AIG] [DAILY, MINUTE]and completed without errors.Backwards compatible for
str(), for types with__repr__(), and for types without aToString()override. Therepr()text of objects whose type overridesToString()changes to that text.CHANGELOG.mdhas an entry under[Unreleased].🤖 Generated with Claude Code