-
Notifications
You must be signed in to change notification settings - Fork 772
Provide hook to implement __repr__ #808
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
Changes from 10 commits
351d9ff
d71a391
2a002d7
cf1f1dc
1c0984f
a5b245b
82aea1f
502f0c3
eb4004a
c6ae6a4
0916406
2fb6a43
9fc5b5f
00b55db
accc797
ff5d96c
264c78b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -233,7 +233,75 @@ public static IntPtr tp_str(IntPtr ob) | |
| } | ||
| try | ||
| { | ||
| //As per python doc: | ||
| //The return value must be a string object. If a class defines __repr__() but not __str__(), | ||
| //then __repr__() is also used when an “informal” string representation of instances of that | ||
| //class is required. | ||
| //In C#, everything provides ToString(), so the check here will be whether the type explicitly | ||
| //provides ToString() or if it is language provided (i.e. the fully qualified type name as a string) | ||
|
|
||
| //First check which type in the object hierarchy provides ToString() | ||
| //ToString has two "official" overloads so loop over GetMethods to get the one without parameters | ||
| var instType = co.inst.GetType(); | ||
| foreach (var method in instType.GetMethods()) | ||
| { | ||
|
|
||
| //TODO this could probably be done more cleanly with Linq | ||
| if (!method.IsPublic) continue; //skip private/protected methods | ||
| if (method.Name != "ToString") continue; //only look for ToString | ||
| if (method.DeclaringType == typeof(object)) continue; //ignore default from object | ||
| if (method.GetParameters().Length != 0) continue; //ignore Formatter overload of ToString | ||
|
|
||
| //match! something other than object provides a parameter-less overload of ToString | ||
| return Runtime.PyString_FromString(co.inst.ToString()); | ||
| } | ||
|
|
||
| //If the object defines __repr__, call it. | ||
| System.Reflection.MethodInfo reprMethodInfo = instType.GetMethod("__repr__"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since all .NET types have
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's not the same, though.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this is the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lostmsu See the comment: This sentence comes straight from the python manual, I didn't make it up
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @koubaa the default inheritance hierarchy for C# classes from Python's point of view is Python's Since for all .NET classes overriding
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lostmsu I took a look. I agree some kind of consensus between interface vs reflection for dunder methods should be reached and applied throughout the code base. One argument for reflection is that you can use extension methods to add these. If I take an existing C# codebase and write some extension methods in a separate assembly, I can provide
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @koubaa, extension methods would be hard to find. You'd have to enumerate all types in all assemblies. In addition to that, what would be the semantics of multiple assemblies defining an extension method with the same signature?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lostmsu ah, didn't know that.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lostmsu I am still not convinced about the interface approach. It would require a hard build dependency on C# libraries onto PythonNet. It seems like a tough sell for some of the mature C# libraries out there.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lostmsu I changed this as you requested. I don't think I can capture the repr function in a Func object since this is a static method. |
||
| if (reprMethodInfo != null && reprMethodInfo.IsPublic) | ||
| { | ||
| var reprString = reprMethodInfo.Invoke(co.inst, null) as string; | ||
| return Runtime.PyString_FromString(reprString); | ||
| } | ||
|
|
||
| //otherwise fallback to object's ToString() implementation | ||
| return Runtime.PyString_FromString(co.inst.ToString()); | ||
|
|
||
| } | ||
| catch (Exception e) | ||
| { | ||
| if (e.InnerException != null) | ||
|
koubaa marked this conversation as resolved.
|
||
| { | ||
| e = e.InnerException; | ||
| } | ||
| Exceptions.SetError(e); | ||
| return IntPtr.Zero; | ||
| } | ||
| } | ||
|
|
||
| public static IntPtr tp_repr(IntPtr ob) | ||
| { | ||
| var co = GetManagedObject(ob) as CLRObject; | ||
| if (co == null) | ||
| { | ||
| return Exceptions.RaiseTypeError("invalid object"); | ||
| } | ||
| try | ||
| { | ||
| //if __repr__ is defined, use it | ||
| var instType = co.inst.GetType(); | ||
| System.Reflection.MethodInfo methodInfo = instType.GetMethod("__repr__"); | ||
| if (methodInfo != null && methodInfo.IsPublic) | ||
| { | ||
| var reprString = methodInfo.Invoke(co.inst, null) as string; | ||
|
koubaa marked this conversation as resolved.
|
||
| return Runtime.PyString_FromString(reprString); | ||
| } | ||
|
|
||
| //otherwise use the standard object.__repr__(inst) | ||
| IntPtr args = Runtime.PyTuple_New(1); | ||
| Runtime.PyTuple_SetItem(args, 0, ob); | ||
| IntPtr reprFunc = Runtime.PyObject_GetAttrString(Runtime.PyBaseObjectType, "__repr__"); | ||
| return Runtime.PyObject_Call(reprFunc, args, IntPtr.Zero); | ||
|
koubaa marked this conversation as resolved.
Outdated
|
||
| } | ||
| catch (Exception e) | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| using System; | ||
| using System.Text; | ||
|
|
||
| namespace Python.Test | ||
| { | ||
| /// <summary> | ||
| /// Supports repr unit tests. | ||
| /// </summary> | ||
| public class ReprTest | ||
| { | ||
| public class Point | ||
| { | ||
| public Point(double x, double y) | ||
| { | ||
| X = x; | ||
| Y = y; | ||
| } | ||
|
|
||
| public double X { get; set; } | ||
| public double Y { get; set; } | ||
|
|
||
| public override string ToString() | ||
| { | ||
| return base.ToString() + ": X=" + X.ToString() + ", Y=" + Y.ToString(); | ||
| } | ||
|
|
||
| public string __repr__() | ||
| { | ||
| return "Point(" + X.ToString() + "," + Y.ToString() + ")"; | ||
| } | ||
| } | ||
|
|
||
| public class Foo | ||
| { | ||
| public string __repr__() | ||
| { | ||
| return "I implement __repr__() but not ToString()!"; | ||
| } | ||
| } | ||
|
|
||
| public class Bar | ||
| { | ||
| public override string ToString() | ||
| { | ||
| return "I implement ToString() but not __repr__()!"; | ||
| } | ||
| } | ||
|
|
||
| public class BazBase | ||
| { | ||
| public override string ToString() | ||
| { | ||
| return "Base class implementing ToString()!"; | ||
| } | ||
| } | ||
|
|
||
| public class BazMiddle : BazBase | ||
| { | ||
| public override string ToString() | ||
| { | ||
| return "Middle class implementing ToString()!"; | ||
| } | ||
| } | ||
|
|
||
| //implements ToString via BazMiddle | ||
| public class Baz : BazMiddle | ||
| { | ||
|
|
||
| } | ||
|
|
||
| public class Quux | ||
| { | ||
| public string ToString(string format) | ||
| { | ||
| return "I implement ToString() with an argument!"; | ||
| } | ||
| } | ||
|
|
||
| public class QuuzBase | ||
| { | ||
| protected string __repr__() | ||
| { | ||
| return "I implement __repr__ but it isn't public!"; | ||
| } | ||
| } | ||
|
|
||
| public class Quuz : QuuzBase | ||
| { | ||
|
|
||
| } | ||
|
|
||
| public class Corge | ||
| { | ||
| public string __repr__(int i) | ||
| { | ||
| return "__repr__ implemention with input parameter!"; | ||
| } | ||
| } | ||
|
|
||
| public class Grault | ||
| { | ||
| public int __repr__() | ||
| { | ||
| return "__repr__ implemention with wrong return type!".Length; | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| # -*- coding: utf-8 -*- | ||
|
|
||
| """Test __repr__ output""" | ||
|
|
||
| import System | ||
| import pytest | ||
| from Python.Test import ReprTest | ||
|
|
||
| def test_basic(): | ||
| """Test Point class which implements both ToString and __repr__ without inheritance""" | ||
| ob = ReprTest.Point(1,2) | ||
| # point implements ToString() and __repr__() | ||
| assert ob.__repr__() == "Point(1,2)" | ||
| assert str(ob) == "Python.Test.ReprTest+Point: X=1, Y=2" | ||
|
|
||
| def test_system_string(): | ||
| """Test system string""" | ||
| ob = System.String("hello") | ||
| assert str(ob) == "hello" | ||
| assert "<System.String object at " in ob.__repr__() | ||
|
|
||
| def test_repr_only(): | ||
| """Test class implementing __repr__() but not ToString()""" | ||
| ob = ReprTest.Foo() | ||
| assert str(ob) == "I implement __repr__() but not ToString()!" | ||
|
koubaa marked this conversation as resolved.
Outdated
|
||
| assert ob.__repr__() == "I implement __repr__() but not ToString()!" | ||
|
|
||
| def test_str_only(): | ||
| """Test class implementing ToString() but not __repr__()""" | ||
| ob = ReprTest.Bar() | ||
| assert str(ob) == "I implement ToString() but not __repr__()!" | ||
| assert "<Python.Test.Bar object at " in ob.__repr__() | ||
|
|
||
| def test_hierarchy1(): | ||
| """Test inheritance heirarchy with base & middle class implementing ToString""" | ||
| ob1 = ReprTest.BazBase() | ||
| assert str(ob1) == "Base class implementing ToString()!" | ||
| assert "<Python.Test.BazBase object at " in ob1.__repr__() | ||
|
|
||
| ob2 = ReprTest.BazMiddle() | ||
| assert str(ob2) == "Middle class implementing ToString()!" | ||
| assert "<Python.Test.BazMiddle object at " in ob2.__repr__() | ||
|
|
||
| ob3 = ReprTest.Baz() | ||
| assert str(ob3) == "Middle class implementing ToString()!" | ||
| assert "<Python.Test.Baz object at " in ob3.__repr__() | ||
|
|
||
| def bad_tostring(): | ||
| """Test ToString that can't be used by str()""" | ||
| ob = ReprTest.Quux() | ||
| assert str(ob) == "Python.Test.ReprTest+Quux" | ||
| assert "<Python.Test.Quux object at " in ob.__repr__() | ||
|
|
||
| def bad_repr(): | ||
| """Test incorrect implementation of repr""" | ||
| ob1 = ReprTest.QuuzBase() | ||
| assert str(ob1) == "Python.Test.ReprTest+QuuzBase" | ||
| assert "<Python.Test.QuuzBase object at " in ob.__repr__() | ||
|
|
||
| ob2 = ReprTest.Quuz() | ||
| assert str(ob2) == "Python.Test.ReprTest+Quuz" | ||
| assert "<Python.Test.Quuz object at " in ob.__repr__() | ||
|
|
||
| ob3 = ReprTest.Corge() | ||
| with pytest.raises(Exception): | ||
| str(ob3) | ||
| with pytest.raises(Exception): | ||
| ob3.__repr__() | ||
|
|
||
| ob4 = ReprTest.Grault() | ||
| with pytest.raises(Exception): | ||
| str(ob4) | ||
| with pytest.raises(Exception): | ||
| ob4.__repr__() | ||
Uh oh!
There was an error while loading. Please reload this page.