Skip to content

Commit

Permalink
Revert "Fix: interface property implementation in python models (#7987)"
Browse files Browse the repository at this point in the history
This reverts commit ab40b5c.
  • Loading branch information
Martin-Molinero committed May 9, 2024
1 parent 53fb994 commit 4f340fc
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 67 deletions.
3 changes: 1 addition & 2 deletions Common/Python/PythonWrapper.cs
Expand Up @@ -46,8 +46,7 @@ public static PyObject ValidateImplementationOf<TInterface>(this PyObject model)
{
foreach (var member in members)
{
if ((member is not MethodInfo method || !method.IsSpecialName) &&
!model.HasAttr(member.Name) && !model.HasAttr(member.Name.ToSnakeCase()))
if (!model.HasAttr(member.Name) && !model.HasAttr(member.Name.ToSnakeCase()))
{
missingMembers.Add(member.Name);
}
Expand Down
96 changes: 31 additions & 65 deletions Tests/Python/PythonWrapperTests.cs
Expand Up @@ -19,7 +19,6 @@
using QuantConnect.Python;
using System.Collections.Generic;
using QuantConnect.Statistics;
using System.Reflection;

namespace QuantConnect.Tests.Python
{
Expand All @@ -28,31 +27,47 @@ public static class PythonWrapperTests
[TestFixture]
public class ValidateImplementationOf
{
[TestCase(nameof(MissingMethodOne), "ModelMissingMethodOne", "MethodOne")]
[TestCase(nameof(MissingProperty), "ModelMissingProperty", "PropertyOne")]
public void ThrowsOnMissingMember(string moduleName, string className, string missingMemberName)
[Test]
public void ThrowsOnMissingMember()
{
using (Py.GIL())
{
var moduleStr = GetFieldValue(moduleName);
var module = PyModule.FromString(nameof(ValidateImplementationOf), moduleStr);
var model = module.GetAttr(className).Invoke();
var module = PyModule.FromString(nameof(ValidateImplementationOf), MissingMethodOne);
var model = module.GetAttr("ModelMissingMethodOne");
Assert.That(() => model.ValidateImplementationOf<IModel>(), Throws
.Exception.InstanceOf<NotImplementedException>().With.Message.Contains(missingMemberName));
.Exception.InstanceOf<NotImplementedException>().With.Message.Contains("MethodOne"));
}
}

[Test]
public void DoesNotThrowWhenInterfaceFullyImplemented()
{
using (Py.GIL())
{
var module = PyModule.FromString(nameof(ValidateImplementationOf), FullyImplemented);
var model = module.GetAttr("FullyImplementedModel");
Assert.That(() => model.ValidateImplementationOf<IModel>(), Throws.Nothing);
}
}

[TestCase(nameof(FullyImplemented), "FullyImplementedModel")]
[TestCase(nameof(FullyImplementedSnakeCase), "FullyImplementedSnakeCaseModel")]
[TestCase(nameof(FullyImplementedWithPropertyAsField), "FullyImplementedModelWithPropertyAsField")]
[TestCase(nameof(DerivedFromCsharp), "DerivedFromCSharpModel")]
public void DoesNotThrowWhenInterfaceFullyImplemented(string moduleName, string className)
[Test]
public void DoesNotThrowWhenInterfaceFullyImplementedSnakeCaseStyle()
{
using (Py.GIL())
{
var moduleStr = GetFieldValue(moduleName);
var module = PyModule.FromString(nameof(ValidateImplementationOf), moduleStr);
var model = module.GetAttr(className).Invoke();
var module = PyModule.FromString(nameof(ValidateImplementationOf), FullyImplementedSnakeCase);
var model = module.GetAttr("FullyImplementedSnakeCaseModel");
Assert.That(() => model.ValidateImplementationOf<IModel>(), Throws.Nothing);
}
}

[Test]
public void DoesNotThrowWhenDerivedFromCSharpModel()
{
using (Py.GIL())
{
var module = PyModule.FromString(nameof(ValidateImplementationOf), DerivedFromCsharp);
var model = module.GetAttr("DerivedFromCSharpModel");
Assert.That(() => model.ValidateImplementationOf<IModel>(), Throws.Nothing);
}
}
Expand Down Expand Up @@ -213,11 +228,6 @@ public void PEP8StyleCustomModelsWork()
);
}

private static string GetFieldValue(string name)
{
return typeof(ValidateImplementationOf).GetField(name, BindingFlags.Static | BindingFlags.NonPublic).GetValue(null) as string;
}

private const string FullyImplemented =
@"
from clr import AddReference
Expand All @@ -230,9 +240,6 @@ class FullyImplementedModel:
pass
def MethodTwo():
pass
@property
def PropertyOne(self):
return 'value'
";

Expand All @@ -248,26 +255,7 @@ class FullyImplementedSnakeCaseModel:
pass
def method_two():
pass
@property
def property_one(self):
pass
";

private const string FullyImplementedWithPropertyAsField =
@"
from clr import AddReference
AddReference('QuantConnect.Tests')
from QuantConnect.Tests.Python import *
class FullyImplementedModelWithPropertyAsField:
def method_one():
pass
def method_two():
pass
def __init__(self):
self.property_one = 'value'
";

private const string DerivedFromCsharp =
Expand All @@ -280,9 +268,6 @@ class FullyImplementedModelWithPropertyAsField:
class DerivedFromCSharpModel(PythonWrapperTests.ValidateImplementationOf.Model):
def MethodOne():
pass
@property
def PropertyOne(self):
return 'value'
";

Expand All @@ -296,36 +281,17 @@ class DerivedFromCSharpModel(PythonWrapperTests.ValidateImplementationOf.Model):
class ModelMissingMethodOne:
def MethodTwo():
pass
@property
def PropertyOne(self):
return 'value'
";
private const string MissingProperty =
@"
from clr import AddReference
AddReference('QuantConnect.Tests')
from QuantConnect.Tests.Python import *
class ModelMissingProperty:
def MethodOne():
pass
def MethodTwo():
pass
";

interface IModel
{
string PropertyOne { get; set; }
void MethodOne();
void MethodTwo();
}

public class Model : IModel
{
public string PropertyOne { get; set; }

public void MethodOne()
{
}
Expand Down

0 comments on commit 4f340fc

Please sign in to comment.