From 720365b9cd5b9269a8cdc9ef9f2f5a6233392c68 Mon Sep 17 00:00:00 2001 From: Aaron Powell Date: Wed, 28 Aug 2024 12:06:24 +1000 Subject: [PATCH 1/2] Reducing the calls to ConvertTo/ConvertFrom so we can refactor them easier --- .../PyObjectTypeConverter.Dictionary.cs | 10 +- .../PyObjectTypeConverter.Generator.cs | 2 +- .../PyObjectTypeConverter.List.cs | 16 +-- .../PyObjectTypeConverter.Tuple.cs | 6 +- .../PyObjectTypeConverter.Utils.cs | 3 +- src/CSnakes.Runtime/PyObjectTypeConverter.cs | 127 ++---------------- src/CSnakes.Runtime/Python/PyObject.cs | 18 ++- 7 files changed, 38 insertions(+), 144 deletions(-) diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.Dictionary.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.Dictionary.cs index 036220a9..a748b41b 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.Dictionary.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.Dictionary.cs @@ -6,7 +6,7 @@ namespace CSnakes.Runtime; internal partial class PyObjectTypeConverter { - private object ConvertToDictionary(PyObject pyObject, Type destinationType, bool useMappingProtocol = false) + private static object ConvertToDictionary(PyObject pyObject, Type destinationType, bool useMappingProtocol = false) { using PyObject items = useMappingProtocol ? PyObject.Create(CPythonAPI.PyMapping_Items(pyObject)) : @@ -67,7 +67,7 @@ private object ConvertToDictionary(PyObject pyObject, Type destinationType, bool return typeInfo.ReturnTypeConstructor.Invoke([dict]); } - internal IReadOnlyDictionary ConvertToDictionary(PyObject pyObject) where TKey : notnull + internal static IReadOnlyDictionary ConvertToDictionary(PyObject pyObject) where TKey : notnull { using PyObject items = PyObject.Create(CPythonAPI.PyMapping_Items(pyObject)); @@ -89,7 +89,7 @@ internal IReadOnlyDictionary ConvertToDictionary(PyO return dict; } - private PyObject ConvertFromDictionary(IDictionary dictionary) + private static PyObject ConvertFromDictionary(IDictionary dictionary) { int len = dictionary.Keys.Count; PyObject[] keys = new PyObject[len]; @@ -98,8 +98,8 @@ private PyObject ConvertFromDictionary(IDictionary dictionary) int i = 0; foreach (DictionaryEntry kvp in dictionary) { - keys[i] = ToPython(kvp.Key); - values[i] = ToPython(kvp.Value); + keys[i] = PyObject.From(kvp.Key); + values[i] = PyObject.From(kvp.Value); i++; } diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.Generator.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.Generator.cs index ceada322..69c564ac 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.Generator.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.Generator.cs @@ -5,7 +5,7 @@ namespace CSnakes.Runtime; internal partial class PyObjectTypeConverter { - internal object ConvertToGeneratorIterator(PyObject pyObject, Type destinationType) + internal static object ConvertToGeneratorIterator(PyObject pyObject, Type destinationType) { if (!CPythonAPI.IsPyGenerator(pyObject)) { diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.List.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.List.cs index 564936b2..fe87a3eb 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.List.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.List.cs @@ -6,7 +6,7 @@ namespace CSnakes.Runtime; internal partial class PyObjectTypeConverter { - private ICollection ConvertToList(PyObject pyObject, Type destinationType) + private static ICollection ConvertToList(PyObject pyObject, Type destinationType) { Type genericArgument = destinationType.GetGenericArguments()[0]; @@ -29,7 +29,7 @@ private ICollection ConvertToList(PyObject pyObject, Type destinationType) return list; } - private ICollection ConvertToListFromSequence(PyObject pyObject, Type destinationType) + private static ICollection ConvertToListFromSequence(PyObject pyObject, Type destinationType) { Type genericArgument = destinationType.GetGenericArguments()[0]; @@ -46,13 +46,13 @@ private ICollection ConvertToListFromSequence(PyObject pyObject, Type destinatio for (var i = 0; i < listSize; i++) { using PyObject item = PyObject.Create(CPythonAPI.PySequence_GetItem(pyObject, i)); - list.Add(ConvertTo(item, genericArgument)); + list.Add(item.As(genericArgument)); } return list; } - internal IReadOnlyCollection ConvertToCollection(PyObject pyObject) + internal static IReadOnlyCollection ConvertToCollection(PyObject pyObject) { nint listSize = CPythonAPI.PySequence_Size(pyObject); var list = new List((int)listSize); @@ -65,25 +65,25 @@ internal IReadOnlyCollection ConvertToCollection(PyObject pyObject return list; } - private PyObject ConvertFromList(ICollection e) + private static PyObject ConvertFromList(ICollection e) { List pyObjects = new(e.Count); foreach (object? item in e) { - pyObjects.Add(ConvertFrom(item)); + pyObjects.Add(PyObject.From(item)); } return Pack.CreateList(CollectionsMarshal.AsSpan(pyObjects)); } - private PyObject ConvertFromList(IEnumerable e) + private static PyObject ConvertFromList(IEnumerable e) { List pyObjects = []; foreach (object? item in e) { - pyObjects.Add(ConvertFrom(item)); + pyObjects.Add(PyObject.From(item)); } return Pack.CreateList(CollectionsMarshal.AsSpan(pyObjects)); diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.Tuple.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.Tuple.cs index e9c1cbf9..5a5574e8 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.Tuple.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.Tuple.cs @@ -6,19 +6,19 @@ namespace CSnakes.Runtime; internal partial class PyObjectTypeConverter { - private PyObject ConvertFromTuple(ITuple t) + private static PyObject ConvertFromTuple(ITuple t) { PyObject[] pyObjects = new PyObject[t.Length]; for (var i = 0; i < t.Length; i++) { - pyObjects[i] = ConvertFrom(t[i]!); // NULL->PyNone + pyObjects[i] = PyObject.From(t[i]); // NULL->PyNone } return Pack.CreateTuple(pyObjects); } - internal ITuple ConvertToTuple(PyObject pyObj, Type destinationType) + internal static ITuple ConvertToTuple(PyObject pyObj, Type destinationType) { if (!CPythonAPI.IsPyTuple(pyObj)) { diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.Utils.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.Utils.cs index 785988ae..2669b0cf 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.Utils.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.Utils.cs @@ -3,8 +3,7 @@ namespace CSnakes.Runtime; internal partial class PyObjectTypeConverter { - private static ConcurrentDictionary<(Type, Type), bool> assignableGenericsMap = []; - + private static readonly ConcurrentDictionary<(Type, Type), bool> assignableGenericsMap = []; public static bool IsAssignableToGenericType(Type givenType, Type genericType) { diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.cs index c504d4a9..ad4985a7 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.cs @@ -9,126 +9,35 @@ namespace CSnakes.Runtime; internal partial class PyObjectTypeConverter { - private readonly ConcurrentDictionary knownDynamicTypes = []; + private static readonly ConcurrentDictionary knownDynamicTypes = []; - /// - /// Convert a Python object to a CLR managed object. - /// Caller should already hold the GIL because this function uses the Python runtime for some conversions. - /// - /// - /// - /// - /// - /// - /// Passed object is not a PyObject - /// Source/Target types do not match - public object ConvertTo(object? value, Type destinationType) + public static object PyObjectToManagedType(PyObject pyObject, Type destinationType) { - if (value is not PyObject pyObject) + if (CPythonAPI.IsPyDict(pyObject) && IsAssignableToGenericType(destinationType, dictionaryType)) { - throw new NotSupportedException(); + return ConvertToDictionary(pyObject, destinationType); } - if (destinationType == pyObjectType) + if (CPythonAPI.IsPyList(pyObject) && IsAssignableToGenericType(destinationType, listType)) { - return pyObject.Clone(); + return ConvertToList(pyObject, destinationType); } - if (destinationType == typeof(byte[]) && CPythonAPI.IsBytes(pyObject)) + // This needs to come after lists, because sequences are also maps + if (CPythonAPI.IsPyMappingWithItems(pyObject) && destinationType.IsAssignableTo(collectionType)) { - return CPythonAPI.PyBytes_AsByteArray(pyObject); + return ConvertToDictionary(pyObject, destinationType, useMappingProtocol: true); } - if (destinationType == typeof(long)) + if (CPythonAPI.IsPySequence(pyObject) && IsAssignableToGenericType(destinationType, listType)) { - long result = CPythonAPI.PyLong_AsLongLong(pyObject); - if (result == -1 && CPythonAPI.PyErr_Occurred()) - { - throw PyObject.ThrowPythonExceptionAsClrException("Error converting Python object to long, check that the object was a Python long or that the value wasn't too large. See InnerException for details."); - } - return result; - } - - if (destinationType == typeof(BigInteger) && CPythonAPI.IsPyLong(pyObject)) - { - return ConvertToBigInteger(pyObject, destinationType); - } - - if (destinationType == typeof(int)) - { - var result = CPythonAPI.PyLong_AsLong(pyObject); - if (result == -1 && CPythonAPI.PyErr_Occurred()) - { - throw PyObject.ThrowPythonExceptionAsClrException("Error converting Python object to int, check that the object was a Python int or that the value wasn't too large. See InnerException for details."); - } - return result; - } - - if (destinationType == typeof(bool) && CPythonAPI.IsPyBool(pyObject)) - { - return CPythonAPI.IsPyTrue(pyObject); - } - - if (destinationType == typeof(double)) - { - var result = CPythonAPI.PyFloat_AsDouble(pyObject); - if (result == -1 && CPythonAPI.PyErr_Occurred()) - { - throw PyObject.ThrowPythonExceptionAsClrException("Error converting Python object to double, check that the object was a Python float. See InnerException for details."); - } - return result; - } - - if (destinationType.IsAssignableTo(typeof(ITuple))) - { - if (CPythonAPI.IsPyTuple(pyObject)) - { - return ConvertToTuple(pyObject, destinationType); - } - - var tupleTypes = destinationType.GetGenericArguments(); - if (tupleTypes.Length > 1) - { - throw new InvalidCastException($"The type is a tuple with more than one generic argument, but the underlying Python type is not a tuple. Destination Type: {destinationType}"); - } - - var convertedValue = ConvertTo(pyObject, tupleTypes[0]); - return Activator.CreateInstance(destinationType, convertedValue)!; - } - - if (destinationType.IsGenericType) - { - if (IsAssignableToGenericType(destinationType, dictionaryType) && CPythonAPI.IsPyDict(pyObject)) - { - return ConvertToDictionary(pyObject, destinationType); - } - - if (IsAssignableToGenericType(destinationType, listType) && CPythonAPI.IsPyList(pyObject)) - { - return ConvertToList(pyObject, destinationType); - } - - if (IsAssignableToGenericType(destinationType, generatorIteratorType) && CPythonAPI.IsPyGenerator(pyObject)) - { - return ConvertToGeneratorIterator(pyObject, destinationType); - } - - // This needs to come after lists, because sequences are also maps - if (destinationType.IsAssignableTo(collectionType) && CPythonAPI.IsPyMappingWithItems(pyObject)) - { - return ConvertToDictionary(pyObject, destinationType, useMappingProtocol: true); - } - - if (IsAssignableToGenericType(destinationType, listType) && CPythonAPI.IsPySequence(pyObject)) - { - return ConvertToListFromSequence(pyObject, destinationType); - } + return ConvertToListFromSequence(pyObject, destinationType); } throw new InvalidCastException($"Attempting to cast {destinationType} from {pyObject.GetPythonType()}"); } - public PyObject ConvertFrom(object value) => + public static PyObject ManagedTypeToPyObject(object value) => value switch { string str => PyObject.Create(CPythonAPI.AsPyUnicodeObject(str)), @@ -147,17 +56,5 @@ public PyObject ConvertFrom(object value) => _ => throw new InvalidCastException($"Cannot convert {value} to PyObject") }; - private PyObject ToPython(object? o) - { - if (o is null) - { - return PyObject.None; - } - - var result = ConvertFrom(o); - - return result is null ? throw new NotImplementedException() : result; - } - record DynamicTypeInfo(ConstructorInfo ReturnTypeConstructor, ConstructorInfo? TransientTypeConstructor = null); } diff --git a/src/CSnakes.Runtime/Python/PyObject.cs b/src/CSnakes.Runtime/Python/PyObject.cs index 76657188..53448e2f 100644 --- a/src/CSnakes.Runtime/Python/PyObject.cs +++ b/src/CSnakes.Runtime/Python/PyObject.cs @@ -11,8 +11,6 @@ namespace CSnakes.Runtime.Python; [DebuggerDisplay("PyObject: repr={GetRepr()}, type={GetPythonType().ToString()}")] public class PyObject : SafeHandle { - private static readonly PyObjectTypeConverter td = new(); - protected PyObject(IntPtr pyObject, bool ownsHandle = true) : base(pyObject, ownsHandle) { if (pyObject == IntPtr.Zero) @@ -454,9 +452,9 @@ internal object As(Type type) var t when t == typeof(string) => CPythonAPI.PyUnicode_AsUTF8(this), var t when t == typeof(BigInteger) => PyObjectTypeConverter.ConvertToBigInteger(this, t), var t when t == typeof(byte[]) => CPythonAPI.PyBytes_AsByteArray(this), - var t when t.IsAssignableTo(typeof(ITuple)) => td.ConvertToTuple(this, t), - var t when t.IsAssignableTo(typeof(IGeneratorIterator)) => td.ConvertToGeneratorIterator(this, t), - var t => td.ConvertTo(this, t), + var t when t.IsAssignableTo(typeof(ITuple)) => PyObjectTypeConverter.ConvertToTuple(this, t), + var t when t.IsAssignableTo(typeof(IGeneratorIterator)) => PyObjectTypeConverter.ConvertToGeneratorIterator(this, t), + var t => PyObjectTypeConverter.PyObjectToManagedType(this, t), }; } } @@ -465,21 +463,21 @@ public IReadOnlyCollection AsCollection() where TColl { using (GIL.Acquire()) { - return td.ConvertToCollection(this); + return PyObjectTypeConverter.ConvertToCollection(this); } } public IReadOnlyDictionary AsDictionary() where TDict : IReadOnlyDictionary where TKey : notnull { using (GIL.Acquire()) { - return td.ConvertToDictionary(this); + return PyObjectTypeConverter.ConvertToDictionary(this); } } public IReadOnlyCollection As() where TCollection : IReadOnlyCollection => - td.ConvertToCollection(this); + PyObjectTypeConverter.ConvertToCollection(this); public IReadOnlyDictionary As() where TDict : IReadOnlyDictionary where TKey : notnull => - td.ConvertToDictionary(this); + PyObjectTypeConverter.ConvertToDictionary(this); public static PyObject From(T value) { @@ -497,7 +495,7 @@ public static PyObject From(T value) double d => Create(CPythonAPI.PyFloat_FromDouble(d)), string s => Create(CPythonAPI.AsPyUnicodeObject(s)), BigInteger bi => PyObjectTypeConverter.ConvertFromBigInteger(bi), - _ => td.ConvertFrom(value), + _ => PyObjectTypeConverter.ManagedTypeToPyObject(value), }; } } From 7c6e1b6139188e5b87c20bb81ff09b39df40083a Mon Sep 17 00:00:00 2001 From: Aaron Powell Date: Wed, 28 Aug 2024 12:11:01 +1000 Subject: [PATCH 2/2] Removing the old 'ConvertFrom' method --- .../PyObjectTypeConverter.Dictionary.cs | 2 +- .../PyObjectTypeConverter.List.cs | 4 ++-- .../PyObjectTypeConverter.Tuple.cs | 2 +- src/CSnakes.Runtime/PyObjectTypeConverter.cs | 19 ------------------- src/CSnakes.Runtime/Python/PyObject.cs | 10 ++++++++-- 5 files changed, 12 insertions(+), 25 deletions(-) diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.Dictionary.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.Dictionary.cs index a748b41b..c8f1c0ef 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.Dictionary.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.Dictionary.cs @@ -89,7 +89,7 @@ internal static IReadOnlyDictionary ConvertToDictionary ConvertToCollection(PyObject p return list; } - private static PyObject ConvertFromList(ICollection e) + internal static PyObject ConvertFromList(ICollection e) { List pyObjects = new(e.Count); @@ -77,7 +77,7 @@ private static PyObject ConvertFromList(ICollection e) return Pack.CreateList(CollectionsMarshal.AsSpan(pyObjects)); } - private static PyObject ConvertFromList(IEnumerable e) + internal static PyObject ConvertFromList(IEnumerable e) { List pyObjects = []; diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.Tuple.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.Tuple.cs index 5a5574e8..a9b9a898 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.Tuple.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.Tuple.cs @@ -6,7 +6,7 @@ namespace CSnakes.Runtime; internal partial class PyObjectTypeConverter { - private static PyObject ConvertFromTuple(ITuple t) + internal static PyObject ConvertFromTuple(ITuple t) { PyObject[] pyObjects = new PyObject[t.Length]; diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.cs index ad4985a7..3ec49e07 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.cs @@ -37,24 +37,5 @@ public static object PyObjectToManagedType(PyObject pyObject, Type destinationTy throw new InvalidCastException($"Attempting to cast {destinationType} from {pyObject.GetPythonType()}"); } - public static PyObject ManagedTypeToPyObject(object value) => - value switch - { - string str => PyObject.Create(CPythonAPI.AsPyUnicodeObject(str)), - byte[] bytes => PyObject.Create(CPythonAPI.PyBytes_FromByteSpan(bytes.AsSpan())), - long l => PyObject.From(l), - int i => PyObject.From(i), - bool b => PyObject.From(b), - double d => PyObject.From(d), - IDictionary dictionary => ConvertFromDictionary(dictionary), - ITuple t => ConvertFromTuple(t), - ICollection l => ConvertFromList(l), - IEnumerable e => ConvertFromList(e), - BigInteger b => ConvertFromBigInteger(b), - PyObject pyObject => pyObject.Clone(), - null => PyObject.None, - _ => throw new InvalidCastException($"Cannot convert {value} to PyObject") - }; - record DynamicTypeInfo(ConstructorInfo ReturnTypeConstructor, ConstructorInfo? TransientTypeConstructor = null); } diff --git a/src/CSnakes.Runtime/Python/PyObject.cs b/src/CSnakes.Runtime/Python/PyObject.cs index 53448e2f..c631651a 100644 --- a/src/CSnakes.Runtime/Python/PyObject.cs +++ b/src/CSnakes.Runtime/Python/PyObject.cs @@ -1,5 +1,6 @@ using CSnakes.Runtime.CPython; using CSnakes.Runtime.Python.Interns; +using System.Collections; using System.Diagnostics; using System.Numerics; using System.Runtime.CompilerServices; @@ -494,8 +495,13 @@ public static PyObject From(T value) long l => Create(CPythonAPI.PyLong_FromLongLong(l)), double d => Create(CPythonAPI.PyFloat_FromDouble(d)), string s => Create(CPythonAPI.AsPyUnicodeObject(s)), - BigInteger bi => PyObjectTypeConverter.ConvertFromBigInteger(bi), - _ => PyObjectTypeConverter.ManagedTypeToPyObject(value), + byte[] bytes => PyObject.Create(CPythonAPI.PyBytes_FromByteSpan(bytes.AsSpan())), + IDictionary dictionary => PyObjectTypeConverter.ConvertFromDictionary(dictionary), + ITuple t => PyObjectTypeConverter.ConvertFromTuple(t), + ICollection l => PyObjectTypeConverter.ConvertFromList(l), + IEnumerable e => PyObjectTypeConverter.ConvertFromList(e), + BigInteger b => PyObjectTypeConverter.ConvertFromBigInteger(b), + _ => throw new InvalidCastException($"Cannot convert {value} to PyObject"), }; } }