Skip to content

Commit

Permalink
Merge pull request #83 from jhonabreul/feature-snake-case-names-binding
Browse files Browse the repository at this point in the history 
PEP8 compliant names binding
  • Loading branch information
@jhonabreul
jhonabreul authored Apr 11, 2024
2 parents a967d46 + b93cab7 commit 2d864c2
Show file tree
Hide file tree
Showing 10 changed files with 791 additions and 26 deletions.
579 changes: 579 additions & 0 deletions src/embed_tests/ClassManagerTests.cs
View file

Large diffs are not rendered by default.

23 changes: 23 additions & 0 deletions src/embed_tests/TestUtil.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
using NUnit.Framework;

using Python.Runtime;

namespace Python.EmbeddingTest
{
[TestFixture]
public class TestUtil
{
[TestCase("TestCamelCaseString", "test_camel_case_string")]
[TestCase("testCamelCaseString", "test_camel_case_string")]
[TestCase("TestCamelCaseString123 ", "test_camel_case_string123")]
[TestCase("_testCamelCaseString123", "_test_camel_case_string123")]
[TestCase("TestCCS", "test_ccs")]
[TestCase("testCCS", "test_ccs")]
[TestCase("CCSTest", "ccs_test")]
[TestCase("test_CamelCaseString", "test_camel_case_string")]
public void ConvertsNameToSnakeCase(string name, string expected)
{
Assert.AreEqual(expected, name.ToSnakeCase());
}
}
}
4 changes: 2 additions & 2 deletions src/perf_tests/Python.PerformanceTests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.*" />
<PackageReference Include="quantconnect.pythonnet" Version="2.0.29" GeneratePathProperty="true">
<PackageReference Include="quantconnect.pythonnet" Version="2.0.30" GeneratePathProperty="true">
<IncludeAssets>compile</IncludeAssets>
</PackageReference>
</ItemGroup>
Expand All @@ -25,7 +25,7 @@
</Target>

<Target Name="CopyBaseline" AfterTargets="Build">
<Copy SourceFiles="$(NuGetPackageRoot)quantconnect.pythonnet\2.0.29\lib\net5.0\Python.Runtime.dll" DestinationFolder="$(OutDir)baseline" />
<Copy SourceFiles="$(NuGetPackageRoot)quantconnect.pythonnet\2.0.30\lib\net5.0\Python.Runtime.dll" DestinationFolder="$(OutDir)baseline" />
</Target>

<Target Name="CopyNewBuild" AfterTargets="Build">
Expand Down
84 changes: 75 additions & 9 deletions src/runtime/ClassManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,18 +336,21 @@ internal static bool ShouldBindEvent(EventInfo ei)
private static ClassInfo GetClassInfo(Type type, ClassBase impl)
{
var ci = new ClassInfo();
var methods = new Dictionary<string, List<MethodBase>>();
var methods = new Dictionary<string, MethodOverloads>();
MethodInfo meth;
ExtensionType ob;
string name;
Type tp;
int i, n;

MemberInfo[] info = type.GetMembers(BindingFlags);
MemberInfo[] info = type.GetMembers(BindingFlags | BindingFlags.FlattenHierarchy);
var local = new HashSet<string>();
var items = new List<MemberInfo>();
MemberInfo m;

var snakeCasedAttributes = new HashSet<string>();
var originalMemberNames = info.Select(mi => mi.Name).ToHashSet();

// Loop through once to find out which names are declared
for (i = 0; i < info.Length; i++)
{
Expand Down Expand Up @@ -430,6 +433,28 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl)
}
}

void CheckForSnakeCasedAttribute(string name)
{
if (snakeCasedAttributes.Remove(name))
{
// If the snake cased attribute is a method, we remove it from the list of methods so that it is not added to the class
methods.Remove(name);
}
}

void AddMember(string name, string snakeCasedName, PyObject obj)
{
CheckForSnakeCasedAttribute(name);

ci.members[name] = obj;

if (!originalMemberNames.Contains(snakeCasedName))
{
ci.members[snakeCasedName] = obj;
snakeCasedAttributes.Add(snakeCasedName);
}
}

for (i = 0; i < items.Count; i++)
{
var mi = (MemberInfo)items[i];
Expand All @@ -448,11 +473,26 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl)
if (name == "__init__" && !impl.HasCustomNew())
continue;

CheckForSnakeCasedAttribute(name);
if (!methods.TryGetValue(name, out var methodList))
{
methodList = methods[name] = new List<MethodBase>();
methodList = methods[name] = new MethodOverloads(true);
}
methodList.Add(meth);

if (!OperatorMethod.IsOperatorMethod(meth))
{
var snakeCasedMethodName = name.ToSnakeCase();
if (snakeCasedMethodName != name && !originalMemberNames.Contains(snakeCasedMethodName))
{
if (!methods.TryGetValue(snakeCasedMethodName, out methodList))
{
methodList = methods[snakeCasedMethodName] = new MethodOverloads(false);
}
methodList.Add(meth);
snakeCasedAttributes.Add(snakeCasedMethodName);
}
}
continue;

case MemberTypes.Constructor when !impl.HasCustomNew():
Expand All @@ -465,7 +505,7 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl)
name = "__init__";
if (!methods.TryGetValue(name, out methodList))
{
methodList = methods[name] = new List<MethodBase>();
methodList = methods[name] = new MethodOverloads(true);
}
methodList.Add(ctor);
continue;
Expand Down Expand Up @@ -493,7 +533,7 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl)
}

ob = new PropertyObject(pi);
ci.members[pi.Name] = ob.AllocObject();
AddMember(pi.Name, pi.Name.ToSnakeCase(), ob.AllocObject());
continue;

case MemberTypes.Field:
Expand All @@ -503,7 +543,14 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl)
continue;
}
ob = new FieldObject(fi);
ci.members[mi.Name] = ob.AllocObject();

var pepName = fi.Name.ToSnakeCase();
if (fi.IsLiteral)
{
pepName = pepName.ToUpper();
}

AddMember(fi.Name, pepName, ob.AllocObject());
continue;

case MemberTypes.Event:
Expand All @@ -515,7 +562,7 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl)
ob = ei.AddMethod.IsStatic
? new EventBinding(ei)
: new EventObject(ei);
ci.members[ei.Name] = ob.AllocObject();
AddMember(ei.Name, ei.Name.ToSnakeCase(), ob.AllocObject());
continue;

case MemberTypes.NestedType:
Expand All @@ -527,6 +574,7 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl)
}
// Note the given instance might be uninitialized
var pyType = GetClass(tp);
CheckForSnakeCasedAttribute(mi.Name);
// make a copy, that could be disposed later
ci.members[mi.Name] = new ReflectedClrType(pyType);
continue;
Expand All @@ -536,9 +584,9 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl)
foreach (var iter in methods)
{
name = iter.Key;
var mlist = iter.Value.ToArray();
var mlist = iter.Value.Methods.ToArray();

ob = new MethodObject(type, name, mlist);
ob = new MethodObject(type, name, mlist, isOriginal: iter.Value.IsOriginal);
ci.members[name] = ob.AllocObject();
if (mlist.Any(OperatorMethod.IsOperatorMethod))
{
Expand Down Expand Up @@ -590,6 +638,24 @@ internal ClassInfo()
indexer = null;
}
}

private class MethodOverloads
{
public List<MethodBase> Methods { get; }

public bool IsOriginal { get; }

public MethodOverloads(bool original = true)
{
Methods = new List<MethodBase>();
IsOriginal = original;
}

public void Add(MethodBase method)
{
Methods.Add(method);
}
}
}

}
44 changes: 37 additions & 7 deletions src/runtime/MethodBinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,15 @@ public int Count
}

internal void AddMethod(MethodBase m)
{
AddMethod(m, true);
}

internal void AddMethod(MethodBase m, bool isOriginal)
{
// we added a new method so we have to re sort the method list
init = false;
list.Add(new MethodInformation(m, m.GetParameters()));
list.Add(new MethodInformation(m, m.GetParameters(), isOriginal));
}

/// <summary>
Expand Down Expand Up @@ -118,7 +123,7 @@ internal static MethodInfo[] MatchParameters(MethodBase[] mi, Type[] tp)
return result.ToArray();
}

// Given a generic method and the argsTypes previously matched with it,
// Given a generic method and the argsTypes previously matched with it,
// generate the matching method
internal static MethodInfo ResolveGenericMethod(MethodInfo method, Object[] args)
{
Expand Down Expand Up @@ -436,6 +441,7 @@ internal Binding Bind(BorrowedReference inst, BorrowedReference args, BorrowedRe
kwArgDict[keyStr!] = new PyObject(value);
}
}
var hasNamedArgs = kwArgDict != null && kwArgDict.Count > 0;

// Fetch our methods we are going to attempt to match and bind too.
var methods = info == null ? GetMethods()
Expand All @@ -447,9 +453,10 @@ internal Binding Bind(BorrowedReference inst, BorrowedReference args, BorrowedRe
// Relevant method variables
var mi = methodInformation.MethodBase;
var pi = methodInformation.ParameterInfo;
// Avoid accessing the parameter names property unless necessary
var paramNames = hasNamedArgs ? methodInformation.ParameterNames : Array.Empty<string>();
int pyArgCount = (int)Runtime.PyTuple_Size(args);


// Special case for operators
bool isOperator = OperatorMethod.IsOperatorMethod(mi);
// Binary operator methods will have 2 CLR args but only one Python arg
Expand Down Expand Up @@ -479,6 +486,7 @@ internal Binding Bind(BorrowedReference inst, BorrowedReference args, BorrowedRe
pyArgCount,
kwArgDict,
pi,
paramNames,
out bool paramsArray,
out ArrayList defaultArgList))
{
Expand All @@ -497,8 +505,8 @@ internal Binding Bind(BorrowedReference inst, BorrowedReference args, BorrowedRe
object arg; // Python -> Clr argument

// Check our KWargs for this parameter
bool hasNamedParam = kwArgDict == null ? false : kwArgDict.TryGetValue(parameter.Name, out tempPyObject);
if(tempPyObject != null)
bool hasNamedParam = kwArgDict == null ? false : kwArgDict.TryGetValue(paramNames[paramIndex], out tempPyObject);
if (tempPyObject != null)
{
op = tempPyObject;
}
Expand Down Expand Up @@ -762,10 +770,16 @@ static BorrowedReference HandleParamsArray(BorrowedReference args, int arrayStar
/// This helper method will perform an initial check to determine if we found a matching
/// method based on its parameters count and type <see cref="Bind(IntPtr,IntPtr,IntPtr,MethodBase,MethodInfo[])"/>
/// </summary>
/// <remarks>
/// We required both the parameters info and the parameters names to perform this check.
/// The CLR method parameters info is required to match the parameters count and type.
/// The names are required to perform an accurate match, since the method can be the snake-cased version.
/// </remarks>
private bool CheckMethodArgumentsMatch(int clrArgCount,
int pyArgCount,
Dictionary<string, PyObject> kwargDict,
ParameterInfo[] parameterInfo,
string[] parameterNames,
out bool paramsArray,
out ArrayList defaultArgList)
{
Expand All @@ -788,7 +802,7 @@ private bool CheckMethodArgumentsMatch(int clrArgCount,
{
// If the method doesn't have all of these kw args, it is not a match
// Otherwise just continue on to see if it is a match
if (!kwargDict.All(x => parameterInfo.Any(pi => x.Key == pi.Name)))
if (!kwargDict.All(x => parameterNames.Any(paramName => x.Key == paramName)))
{
return false;
}
Expand All @@ -808,7 +822,7 @@ private bool CheckMethodArgumentsMatch(int clrArgCount,
defaultArgList = new ArrayList();
for (var v = pyArgCount; v < clrArgCount && match; v++)
{
if (kwargDict != null && kwargDict.ContainsKey(parameterInfo[v].Name))
if (kwargDict != null && kwargDict.ContainsKey(parameterNames[v]))
{
// we have a keyword argument for this parameter,
// no need to check for a default parameter, but put a null
Expand Down Expand Up @@ -973,14 +987,30 @@ internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference a
[Serializable]
internal class MethodInformation
{
private Lazy<string[]> _parametersNames;

public MethodBase MethodBase { get; }

public ParameterInfo[] ParameterInfo { get; }

public bool IsOriginal { get; }

public string[] ParameterNames { get { return _parametersNames.Value; } }

public MethodInformation(MethodBase methodBase, ParameterInfo[] parameterInfo)
: this(methodBase, parameterInfo, true)
{
}

public MethodInformation(MethodBase methodBase, ParameterInfo[] parameterInfo, bool isOriginal)
{
MethodBase = methodBase;
ParameterInfo = parameterInfo;
IsOriginal = isOriginal;

_parametersNames = new Lazy<string[]>(() => IsOriginal
? ParameterInfo.Select(pi => pi.Name).ToArray()
: ParameterInfo.Select(pi => pi.Name.ToSnakeCase()).ToArray());
}

public override string ToString()
Expand Down
4 changes: 2 additions & 2 deletions src/runtime/Properties/AssemblyInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
[assembly: InternalsVisibleTo("Python.EmbeddingTest, PublicKey=00240000048000009400000006020000002400005253413100040000110000005ffd8f49fb44ab0641b3fd8d55e749f716e6dd901032295db641eb98ee46063cbe0d4a1d121ef0bc2af95f8a7438d7a80a3531316e6b75c2dae92fb05a99f03bf7e0c03980e1c3cfb74ba690aca2f3339ef329313bcc5dccced125a4ffdc4531dcef914602cd5878dc5fbb4d4c73ddfbc133f840231343e013762884d6143189")]
[assembly: InternalsVisibleTo("Python.Test, PublicKey=00240000048000009400000006020000002400005253413100040000110000005ffd8f49fb44ab0641b3fd8d55e749f716e6dd901032295db641eb98ee46063cbe0d4a1d121ef0bc2af95f8a7438d7a80a3531316e6b75c2dae92fb05a99f03bf7e0c03980e1c3cfb74ba690aca2f3339ef329313bcc5dccced125a4ffdc4531dcef914602cd5878dc5fbb4d4c73ddfbc133f840231343e013762884d6143189")]

[assembly: AssemblyVersion("2.0.29")]
[assembly: AssemblyFileVersion("2.0.29")]
[assembly: AssemblyVersion("2.0.30")]
[assembly: AssemblyFileVersion("2.0.30")]
2 changes: 1 addition & 1 deletion src/runtime/Python.Runtime.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<RootNamespace>Python.Runtime</RootNamespace>
<AssemblyName>Python.Runtime</AssemblyName>
<PackageId>QuantConnect.pythonnet</PackageId>
<Version>2.0.29</Version>
<Version>2.0.30</Version>
<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
<PackageLicenseFile>LICENSE</PackageLicenseFile>
<RepositoryUrl>https://github.com/pythonnet/pythonnet</RepositoryUrl>
Expand Down
5 changes: 3 additions & 2 deletions src/runtime/Types/MethodObject.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ internal class MethodObject : ExtensionType
internal PyString? doc;
internal MaybeType type;

public MethodObject(MaybeType type, string name, MethodBase[] info, bool allow_threads = MethodBinder.DefaultAllowThreads)
public MethodObject(MaybeType type, string name, MethodBase[] info, bool allow_threads = MethodBinder.DefaultAllowThreads,
bool isOriginal = true)
{
this.type = type;
this.name = name;
Expand All @@ -37,7 +38,7 @@ public MethodObject(MaybeType type, string name, MethodBase[] info, bool allow_t
foreach (MethodBase item in info)
{
this.infoList.Add(item);
binder.AddMethod(item);
binder.AddMethod(item, isOriginal);
if (item.IsStatic)
{
this.is_static = true;
Expand Down
Loading

0 comments on commit 2d864c2

Please sign in to comment.