Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
- Luke Stratman ([@lstratman](https://github.com/lstratman))
- Konstantin Posudevskiy ([@konstantin-posudevskiy](https://github.com/konstantin-posudevskiy))
- Matthias Dittrich ([@matthid](https://github.com/matthid))
- Mohamed Koubaa ([@koubaa](https://github.com/koubaa))
- Patrick Stewart ([@patstew](https://github.com/patstew))
- Raphael Nestler ([@rnestler](https://github.com/rnestler))
- Rickard Holmberg ([@rickardraysearch](https://github.com/rickardraysearch))
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ This document follows the conventions laid out in [Keep a CHANGELOG][].
- Added PyObject finalizer support, Python objects referred by C# can be auto collect now ([#692][p692]).
- Added detailed comments about aproaches and dangers to handle multi-app-domains ([#625][p625])
- Python 3.7 support, builds and testing added. Defaults changed from Python 3.6 to 3.7 ([#698][p698])
- Added support for C# types to provide `__repr__` ([#680][p680])

### Changed

Expand Down
62 changes: 62 additions & 0 deletions src/runtime/classbase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,69 @@ 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();
var method = instType.GetMethod("ToString", new Type[]{});
if (method.DeclaringType != typeof(object))
{
//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__");
Copy link
Copy Markdown
Member

@lostmsu lostmsu Sep 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since all .NET types have ToString, why does it even try __repr__? If somebody really needs it, they can do override string ToString() => __repr__()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not the same, though. ToString is more akin to __str__, there is no real equivalent to __repr__ in .NET.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is the tp_str implementation. tp_repr is below

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lostmsu See the comment:
//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.

This sentence comes straight from the python manual, I didn't make it up

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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 object <- System.Object <- CustomCSharpClass.

Since for all .NET classes overriding ToString is how you implement __str__, I don't see why it should be different for System.Object. Hence I think .NET classes should just always call ToString for tp_str.

Copy link
Copy Markdown
Contributor Author

@koubaa koubaa Sep 16, 2019

Choose a reason for hiding this comment

The 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 __repr__ and __getattr__ to it. What do you think?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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. GetMethods won't return them.

In addition to that, what would be the semantics of multiple assemblies defining an extension method with the same signature?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lostmsu ah, didn't know that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 = (string)reprMethodInfo.Invoke(co.inst, null);
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)
Comment thread
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;
Comment thread
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);
Comment thread
koubaa marked this conversation as resolved.
Outdated
}
catch (Exception e)
{
Expand Down
23 changes: 23 additions & 0 deletions src/runtime/exceptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,29 @@ internal static Exception ToException(IntPtr ob)
return e;
}

/// <summary>
/// Exception __repr__ implementation
/// </summary>
public new static IntPtr tp_repr(IntPtr ob)
{
Exception e = ToException(ob);
if (e == null)
{
return Exceptions.RaiseTypeError("invalid object");
}
string name = e.GetType().Name;
string message;
if (e.Message != String.Empty)
{
message = String.Format("{0}('{1}')", name, e.Message);
}
else
{
message = String.Format("{0}()", name);
}
return Runtime.PyUnicode_FromString(message);
}

/// <summary>
/// Exception __str__ implementation
/// </summary>
Expand Down
3 changes: 2 additions & 1 deletion src/testing/Python.Test.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
<Compile Include="threadtest.cs" />
<Compile Include="doctest.cs" />
<Compile Include="subclasstest.cs" />
<Compile Include="ReprTest.cs" />
</ItemGroup>
<ItemGroup>
<Reference Include="Microsoft.CSharp" />
Expand All @@ -111,4 +112,4 @@
<Copy SourceFiles="$(TargetAssembly)" DestinationFolder="$(SolutionDir)\src\tests\fixtures" />
<!--Copy SourceFiles="$(TargetAssemblyPdb)" Condition="Exists('$(TargetAssemblyPdb)')" DestinationFolder="$(PythonBuildDir)" /-->
</Target>
</Project>
</Project>
108 changes: 108 additions & 0 deletions src/testing/ReprTest.cs
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;
}
}
}
}
7 changes: 2 additions & 5 deletions src/tests/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,11 +282,8 @@ def test_python_compat_of_managed_exceptions():

assert e.args == (msg,)
assert isinstance(e.args, tuple)
if PY3:
strexp = "OverflowException('Simple message"
assert repr(e)[:len(strexp)] == strexp
elif PY2:
assert repr(e) == "OverflowException(u'Simple message',)"
strexp = "OverflowException('Simple message"
assert repr(e)[:len(strexp)] == strexp


def test_exception_is_instance_of_system_object():
Expand Down
74 changes: 74 additions & 0 deletions src/tests/test_repr.py
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()!"
Comment thread
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__()
1 change: 1 addition & 0 deletions src/tests/tests.pyproj
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
<Compile Include="test_recursive_types.py" />
<Compile Include="test_subclass.py" />
<Compile Include="test_thread.py" />
<Compile Include="test_repr.py" />
<Compile Include="utils.py" />
<Compile Include="fixtures\argv-fixture.py" />
</ItemGroup>
Expand Down