From 65c25a14162fb4462d35529be40fac173f7df028 Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Wed, 21 Aug 2024 13:51:03 +1000 Subject: [PATCH 01/11] Add IsNone and None static attribute for PyObject --- src/CSnakes.Runtime/CPython/None.cs | 9 ++++++- src/CSnakes.Runtime/PyObjectTypeConverter.cs | 5 ++-- src/CSnakes.Runtime/Python/PyObject.cs | 20 +++++++++++++- src/CSnakes/Reflection/MethodReflection.cs | 27 ++++++++++++------- src/CSnakes/Reflection/TypeReflection.cs | 5 ++-- .../Integration.Tests.csproj | 4 +++ src/Integration.Tests/NoneTests.cs | 23 ++++++++++++++++ src/Integration.Tests/python/test_none.py | 5 ++++ 8 files changed, 81 insertions(+), 17 deletions(-) create mode 100644 src/Integration.Tests/NoneTests.cs create mode 100644 src/Integration.Tests/python/test_none.py diff --git a/src/CSnakes.Runtime/CPython/None.cs b/src/CSnakes.Runtime/CPython/None.cs index eb3fb93c..ffe47a31 100644 --- a/src/CSnakes.Runtime/CPython/None.cs +++ b/src/CSnakes.Runtime/CPython/None.cs @@ -1,4 +1,6 @@ -namespace CSnakes.Runtime.CPython; +using CSnakes.Runtime.Python; + +namespace CSnakes.Runtime.CPython; internal unsafe partial class CPythonAPI { @@ -13,4 +15,9 @@ internal static nint GetNone() Py_IncRefRaw(PyNone); return PyNone; } + + internal static bool IsNone(PyObject o) + { + return PyNone == o.DangerousGetHandle(); + } } diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.cs index e38ae6f6..3d82da22 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.cs @@ -136,7 +136,8 @@ internal partial class PyObjectTypeConverter : TypeConverter ITuple t => ConvertFromTuple(context, culture, t), IEnumerable e => ConvertFromList(context, culture, e), BigInteger b => ConvertFromBigInteger(context, culture, b), - null => new PyObject(CPythonAPI.GetNone()), + PyObject pyObject => pyObject, + null => PyObject.None, _ => base.ConvertFrom(context, culture, value) }; @@ -179,7 +180,7 @@ private PyObject ToPython(object? o, ITypeDescriptorContext? context, CultureInf { if (o is null) { - return new PyObject(CPythonAPI.GetNone()); + return PyObject.None; } var result = ConvertFrom(context, culture, o); diff --git a/src/CSnakes.Runtime/Python/PyObject.cs b/src/CSnakes.Runtime/Python/PyObject.cs index d9cf334f..a1bb2e4b 100644 --- a/src/CSnakes.Runtime/Python/PyObject.cs +++ b/src/CSnakes.Runtime/Python/PyObject.cs @@ -157,6 +157,24 @@ public string GetRepr() } } + /// + /// Is the Python object None? + /// + /// true if None, else false + public bool IsNone() + { + return CPythonAPI.IsNone(this); + } + + public static PyObject None + { + get + { + // TODO: make none static. + return new PyObject(CPythonAPI.GetNone()); + } + } + /// /// Call the object. Equivalent to (__call__)(args) /// All arguments are treated as positional. @@ -292,7 +310,7 @@ public T As() using (GIL.Acquire()) { return value is null ? - new PyObject(CPythonAPI.GetNone()) : + PyObject.None : (PyObject?)td.ConvertFrom(value); } } diff --git a/src/CSnakes/Reflection/MethodReflection.cs b/src/CSnakes/Reflection/MethodReflection.cs index 58244ff5..a2fbb3b1 100644 --- a/src/CSnakes/Reflection/MethodReflection.cs +++ b/src/CSnakes/Reflection/MethodReflection.cs @@ -51,6 +51,21 @@ public static MethodDefinition FromMethod(PythonFunctionDefinition function, str { continue; } + bool needsConversion = true; // TODO: Skip .From for PyObject arguments. + ExpressionSyntax rhs = IdentifierName(parameter.cSharpParameter.Identifier); + if (needsConversion) + rhs = + InvocationExpression( + MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + IdentifierName("PyObject"), + IdentifierName("From"))) + .WithArgumentList( + ArgumentList( + SingletonSeparatedList( + Argument( + IdentifierName(parameter.cSharpParameter.Identifier))))); + pythonConversionStatements.Add( LocalDeclarationStatement( VariableDeclaration( @@ -63,16 +78,8 @@ public static MethodDefinition FromMethod(PythonFunctionDefinition function, str EqualsValueClause( PostfixUnaryExpression( SyntaxKind.SuppressNullableWarningExpression, - InvocationExpression( - MemberAccessExpression( - SyntaxKind.SimpleMemberAccessExpression, - IdentifierName("PyObject"), - IdentifierName("From"))) - .WithArgumentList( - ArgumentList( - SingletonSeparatedList( - Argument( - IdentifierName(parameter.cSharpParameter.Identifier))))))))))) + rhs + )))))) .WithUsingKeyword( Token(SyntaxKind.UsingKeyword))); } diff --git a/src/CSnakes/Reflection/TypeReflection.cs b/src/CSnakes/Reflection/TypeReflection.cs index 4941737a..36b6610f 100644 --- a/src/CSnakes/Reflection/TypeReflection.cs +++ b/src/CSnakes/Reflection/TypeReflection.cs @@ -26,7 +26,7 @@ public static TypeSyntax AsPredefinedType(PythonTypeSpec pythonType) "Optional" => AsPredefinedType(pythonType.Arguments[0]), "Generator" => CreateGeneratorType(pythonType.Arguments[0], pythonType.Arguments[1], pythonType.Arguments[2]), // Todo more types... see https://docs.python.org/3/library/stdtypes.html#standard-generic-classes - _ => SyntaxFactory.ParseTypeName("PyObject"),// TODO : Should be nullable? + _ => SyntaxFactory.ParseTypeName("PyObject"), }; } return pythonType.Name switch @@ -36,8 +36,7 @@ public static TypeSyntax AsPredefinedType(PythonTypeSpec pythonType) "float" => SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.DoubleKeyword)), "bool" => SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.BoolKeyword)), "bytes" => SyntaxFactory.ParseTypeName("byte[]"), - // Todo more types... - _ => SyntaxFactory.ParseTypeName("PyObject"),// TODO : Should be nullable? + _ => SyntaxFactory.ParseTypeName("PyObject"), }; } diff --git a/src/Integration.Tests/Integration.Tests.csproj b/src/Integration.Tests/Integration.Tests.csproj index 83421d99..8ede156a 100644 --- a/src/Integration.Tests/Integration.Tests.csproj +++ b/src/Integration.Tests/Integration.Tests.csproj @@ -17,6 +17,7 @@ + @@ -49,6 +50,9 @@ Always + + Always + Always diff --git a/src/Integration.Tests/NoneTests.cs b/src/Integration.Tests/NoneTests.cs new file mode 100644 index 00000000..9a5b13fc --- /dev/null +++ b/src/Integration.Tests/NoneTests.cs @@ -0,0 +1,23 @@ +using CSnakes.Runtime.Python; + +namespace Integration.Tests; +public class NoneTests : IntegrationTestBase +{ + [Fact] + public void TestReturnsNoneIsNone() + { + var mod = Env.TestNone(); + using PyObject result = mod.ReturnsNone(); + Assert.True(result.IsNone()); + } + + [Fact] + public void TestNullArgAsNone() + { + PyObject none = PyObject.None; + Assert.True(none.IsNone()); + // Give to function + var mod = Env.TestNone(); + Assert.True(mod.TestNoneResult(none)); + } +} diff --git a/src/Integration.Tests/python/test_none.py b/src/Integration.Tests/python/test_none.py new file mode 100644 index 00000000..3806a627 --- /dev/null +++ b/src/Integration.Tests/python/test_none.py @@ -0,0 +1,5 @@ +def returns_none(): + return None + +def test_none_result(arg) -> bool: + return arg is None From ac94aaca9ffbe0ab54a20309426e572ec219a8cb Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Wed, 21 Aug 2024 14:09:27 +1000 Subject: [PATCH 02/11] Add an immortal PyObject base class and override the None methods. Store a static None instance in PyObject so it can be referenced easily. --- src/CSnakes.Runtime/CPython/Init.cs | 2 ++ src/CSnakes.Runtime/CPython/None.cs | 5 ++++ .../Python/Interns/ImmortalPyObject.cs | 10 ++++++++ .../Python/Interns/PyNoneObject.cs | 25 +++++++++++++++++++ src/CSnakes.Runtime/Python/PyObject.cs | 18 +++++++------ 5 files changed, 52 insertions(+), 8 deletions(-) create mode 100644 src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs create mode 100644 src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs diff --git a/src/CSnakes.Runtime/CPython/Init.cs b/src/CSnakes.Runtime/CPython/Init.cs index c32d54e6..8130bbed 100644 --- a/src/CSnakes.Runtime/CPython/Init.cs +++ b/src/CSnakes.Runtime/CPython/Init.cs @@ -1,4 +1,5 @@ using CSnakes.Runtime.Python; +using CSnakes.Runtime.Python.Interns; using System.Diagnostics; using System.Reflection; using System.Runtime.InteropServices; @@ -83,6 +84,7 @@ private void InitializeEmbeddedPython() // Import builtins module var builtinsMod = Import("builtins"); PyNone = GetAttr(builtinsMod, "None"); + PyObject.none = new PyNoneObject(); Py_DecRef(builtinsMod); } PyEval_SaveThread(); diff --git a/src/CSnakes.Runtime/CPython/None.cs b/src/CSnakes.Runtime/CPython/None.cs index ffe47a31..d285f369 100644 --- a/src/CSnakes.Runtime/CPython/None.cs +++ b/src/CSnakes.Runtime/CPython/None.cs @@ -1,4 +1,5 @@ using CSnakes.Runtime.Python; +using System.ComponentModel; namespace CSnakes.Runtime.CPython; @@ -12,6 +13,10 @@ internal unsafe partial class CPythonAPI /// A new reference to None. In newer versions of Python, None is immortal anyway. internal static nint GetNone() { + if (PyNone == IntPtr.Zero) + { + throw new InvalidOperationException("Python is not initialized. You cannot call this method outside of a Python Environment context."); + } Py_IncRefRaw(PyNone); return PyNone; } diff --git a/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs b/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs new file mode 100644 index 00000000..e897e839 --- /dev/null +++ b/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs @@ -0,0 +1,10 @@ +using CSnakes.Runtime.CPython; +namespace CSnakes.Runtime.Python.Interns; + +internal class ImmortalPyObject(nint handle) : PyObject(handle) +{ + protected override bool ReleaseHandle() + { + return true; + } +} diff --git a/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs b/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs new file mode 100644 index 00000000..07ca6c22 --- /dev/null +++ b/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs @@ -0,0 +1,25 @@ +using CSnakes.Runtime.CPython; + +namespace CSnakes.Runtime.Python.Interns; + +internal class PyNoneObject : ImmortalPyObject +{ + public PyNoneObject() : base(CPythonAPI.GetNone()) + { + } + + public override bool IsNone() + { + return true; + } + + public override string GetRepr() + { + return "None"; + } + + public override string ToString() + { + return "None"; + } +} diff --git a/src/CSnakes.Runtime/Python/PyObject.cs b/src/CSnakes.Runtime/Python/PyObject.cs index a1bb2e4b..f21b031f 100644 --- a/src/CSnakes.Runtime/Python/PyObject.cs +++ b/src/CSnakes.Runtime/Python/PyObject.cs @@ -11,6 +11,7 @@ namespace CSnakes.Runtime.Python; public class PyObject : SafeHandle { private static readonly TypeConverter td = TypeDescriptor.GetConverter(typeof(PyObject)); + internal static PyObject? none; internal PyObject(IntPtr pyObject, bool ownsHandle = true) : base(pyObject, ownsHandle) { @@ -97,7 +98,7 @@ private static void RaiseOnPythonNotInitialized() /// Get the type for the object. /// /// A new reference to the type field. - public PyObject GetPythonType() + public virtual PyObject GetPythonType() { RaiseOnPythonNotInitialized(); using (GIL.Acquire()) @@ -111,7 +112,7 @@ public PyObject GetPythonType() /// /// /// Attribute object (new ref) - public PyObject GetAttr(string name) + public virtual PyObject GetAttr(string name) { RaiseOnPythonNotInitialized(); using (GIL.Acquire()) @@ -120,7 +121,7 @@ public PyObject GetAttr(string name) } } - public bool HasAttr(string name) + public virtual bool HasAttr(string name) { RaiseOnPythonNotInitialized(); using (GIL.Acquire()) @@ -133,7 +134,7 @@ public bool HasAttr(string name) /// Get the iterator for the object. This is equivalent to iter(obj) in Python. /// /// The iterator object (new ref) - public PyObject GetIter() + public virtual PyObject GetIter() { RaiseOnPythonNotInitialized(); using (GIL.Acquire()) @@ -146,7 +147,7 @@ public PyObject GetIter() /// Get the results of the repr() function on the object. /// /// - public string GetRepr() + public virtual string GetRepr() { RaiseOnPythonNotInitialized(); using (GIL.Acquire()) @@ -161,7 +162,7 @@ public string GetRepr() /// Is the Python object None? /// /// true if None, else false - public bool IsNone() + public virtual bool IsNone() { return CPythonAPI.IsNone(this); } @@ -170,8 +171,9 @@ public static PyObject None { get { - // TODO: make none static. - return new PyObject(CPythonAPI.GetNone()); + if (none is null) + throw new InvalidOperationException("Python is not initialized. You cannot call this method outside of a Python Environment context."); + return none; } } From 3f35ccb6960f81a6f619d9dbfcdab0efe2c3bdcf Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Wed, 21 Aug 2024 14:21:44 +1000 Subject: [PATCH 03/11] Override dispose --- src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs b/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs index e897e839..d8d5abe0 100644 --- a/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs +++ b/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs @@ -7,4 +7,9 @@ protected override bool ReleaseHandle() { return true; } + + protected override void Dispose(bool disposing) + { + // I am immortal!! + } } From 11cf3dda4373751a0f7f08cbd9a3411d9bf23cc1 Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Wed, 21 Aug 2024 14:50:35 +1000 Subject: [PATCH 04/11] Update src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs Co-authored-by: Aaron Powell --- .../Python/Interns/PyNoneObject.cs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs b/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs index 07ca6c22..c0b68c05 100644 --- a/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs +++ b/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs @@ -8,18 +8,10 @@ public PyNoneObject() : base(CPythonAPI.GetNone()) { } - public override bool IsNone() - { - return true; - } + public override bool IsNone() => true; - public override string GetRepr() - { - return "None"; - } + public override string GetRepr() => ToString(); + + public override string ToString() => "None"; - public override string ToString() - { - return "None"; - } } From e5741a367df4cd81ea0faa38fef01a96bdc495a3 Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Wed, 21 Aug 2024 14:52:16 +1000 Subject: [PATCH 05/11] Return None static whenever you try and construct a PyObject with a ptr to the None immortal --- src/CSnakes.Runtime/CPython/Import.cs | 2 +- .../PyObjectTypeConverter.BigInteger.cs | 4 ++-- .../PyObjectTypeConverter.Dictionary.cs | 10 +++++----- src/CSnakes.Runtime/PyObjectTypeConverter.List.cs | 6 +++--- src/CSnakes.Runtime/PyObjectTypeConverter.Tuple.cs | 2 +- src/CSnakes.Runtime/PyObjectTypeConverter.cs | 14 +++++++------- .../Python/Interns/ImmortalPyObject.cs | 6 +++++- src/CSnakes.Runtime/Python/PyObject.cs | 11 ++++++++++- src/CSnakes.Runtime/Python/PyTuple.cs | 2 +- 9 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/CSnakes.Runtime/CPython/Import.cs b/src/CSnakes.Runtime/CPython/Import.cs index ff8f4f0c..bfca77ef 100644 --- a/src/CSnakes.Runtime/CPython/Import.cs +++ b/src/CSnakes.Runtime/CPython/Import.cs @@ -15,7 +15,7 @@ internal static PyObject Import(string name) nint pyName = AsPyUnicodeObject(name); nint module = PyImport_Import(pyName); Py_DecRefRaw(pyName); - return new(module); + return PyObject.Create(module); } /// diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.BigInteger.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.BigInteger.cs index f015442d..d532eca6 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.BigInteger.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.BigInteger.cs @@ -15,7 +15,7 @@ internal partial class PyObjectTypeConverter private PyObject ConvertFromBigInteger(ITypeDescriptorContext? context, CultureInfo? culture, BigInteger integer) { - using PyObject pyUnicode = new PyObject(CPythonAPI.AsPyUnicodeObject(integer.ToString())); - return new PyObject(CPythonAPI.PyLong_FromUnicodeObject(pyUnicode, 10)); + using PyObject pyUnicode = PyObject.Create(CPythonAPI.AsPyUnicodeObject(integer.ToString())); + return PyObject.Create(CPythonAPI.PyLong_FromUnicodeObject(pyUnicode, 10)); } } \ No newline at end of file diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.Dictionary.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.Dictionary.cs index 9b0ac662..d08277e1 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.Dictionary.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.Dictionary.cs @@ -10,7 +10,7 @@ internal partial class PyObjectTypeConverter { private object? ConvertToDictionary(PyObject pyObject, Type destinationType, ITypeDescriptorContext? context, CultureInfo? culture, bool useMappingProtocol = false) { - using PyObject items = useMappingProtocol ? new(CPythonAPI.PyMapping_Items(pyObject)) : new(CPythonAPI.PyDict_Items(pyObject)); + using PyObject items = useMappingProtocol ? PyObject.Create(CPythonAPI.PyMapping_Items(pyObject)) : PyObject.Create(CPythonAPI.PyDict_Items(pyObject)); Type item1Type = destinationType.GetGenericArguments()[0]; Type item2Type = destinationType.GetGenericArguments()[1]; Type dictType = typeof(Dictionary<,>).MakeGenericType(item1Type, item2Type); @@ -19,10 +19,10 @@ internal partial class PyObjectTypeConverter for (nint i = 0; i < itemsLength; i++) { - using PyObject item = new(CPythonAPI.PyList_GetItem(items, i)); + using PyObject item = PyObject.Create(CPythonAPI.PyList_GetItem(items, i)); - using PyObject item1 = new(CPythonAPI.PyTuple_GetItem(item, 0)); - using PyObject item2 = new(CPythonAPI.PyTuple_GetItem(item, 1)); + using PyObject item1 = PyObject.Create(CPythonAPI.PyTuple_GetItem(item, 0)); + using PyObject item2 = PyObject.Create(CPythonAPI.PyTuple_GetItem(item, 1)); object? convertedItem1 = AsManagedObject(item1Type, item1, context, culture); object? convertedItem2 = AsManagedObject(item2Type, item2, context, culture); @@ -36,7 +36,7 @@ internal partial class PyObjectTypeConverter private PyObject ConvertFromDictionary(ITypeDescriptorContext? context, CultureInfo? culture, IDictionary dictionary) { - PyObject pyDict = new(CPythonAPI.PyDict_New()); + PyObject pyDict = PyObject.Create(CPythonAPI.PyDict_New()); foreach (DictionaryEntry kvp in dictionary) { diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.List.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.List.cs index 48c127a0..2a1215be 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.List.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.List.cs @@ -15,7 +15,7 @@ internal partial class PyObjectTypeConverter IList list = (IList)Activator.CreateInstance(listType)!; for (var i = 0; i < CPythonAPI.PyList_Size(pyObject); i++) { - using PyObject item = new(CPythonAPI.PyList_GetItem(pyObject, i)); + using PyObject item = PyObject.Create(CPythonAPI.PyList_GetItem(pyObject, i)); list.Add(AsManagedObject(genericArgument, item, context, culture)); } @@ -30,7 +30,7 @@ internal partial class PyObjectTypeConverter IList list = (IList)Activator.CreateInstance(listType)!; for (var i = 0; i < CPythonAPI.PySequence_Size(pyObject); i++) { - using PyObject item = new(CPythonAPI.PySequence_GetItem(pyObject, i)); + using PyObject item = PyObject.Create(CPythonAPI.PySequence_GetItem(pyObject, i)); list.Add(AsManagedObject(genericArgument, item, context, culture)); } @@ -39,7 +39,7 @@ internal partial class PyObjectTypeConverter private PyObject ConvertFromList(ITypeDescriptorContext? context, CultureInfo? culture, IEnumerable e) { - PyObject pyList = new(CPythonAPI.PyList_New(0)); + PyObject pyList = PyObject.Create(CPythonAPI.PyList_New(0)); foreach (var item in e) { diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.Tuple.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.Tuple.cs index b5ff8e1e..2e940a42 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.Tuple.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.Tuple.cs @@ -35,7 +35,7 @@ private PyObject ConvertFromTuple(ITypeDescriptorContext? context, CultureInfo? var tupleValues = new List(); for (nint i = 0; i < CPythonAPI.PyTuple_Size(pyObj); i++) { - PyObject value = new(CPythonAPI.PyTuple_GetItem(pyObj, i)); + PyObject value = PyObject.Create(CPythonAPI.PyTuple_GetItem(pyObj, i)); tupleValues.Add(value); } diff --git a/src/CSnakes.Runtime/PyObjectTypeConverter.cs b/src/CSnakes.Runtime/PyObjectTypeConverter.cs index 3d82da22..54fc2376 100644 --- a/src/CSnakes.Runtime/PyObjectTypeConverter.cs +++ b/src/CSnakes.Runtime/PyObjectTypeConverter.cs @@ -126,17 +126,17 @@ internal partial class PyObjectTypeConverter : TypeConverter public override object? ConvertFrom(ITypeDescriptorContext? context, CultureInfo? culture, object value) => value switch { - string str => new PyObject(CPythonAPI.AsPyUnicodeObject(str)), - byte[] bytes => new PyObject(CPythonAPI.PyBytes_FromByteSpan(bytes.AsSpan())), - long l => new PyObject(CPythonAPI.PyLong_FromLongLong(l)), - int i => new PyObject(CPythonAPI.PyLong_FromLong(i)), - bool b => new PyObject(CPythonAPI.PyBool_FromLong(b ? 1 : 0)), - double d => new PyObject(CPythonAPI.PyFloat_FromDouble(d)), + string str => PyObject.Create(CPythonAPI.AsPyUnicodeObject(str)), + byte[] bytes => PyObject.Create(CPythonAPI.PyBytes_FromByteSpan(bytes.AsSpan())), + long l => PyObject.Create(CPythonAPI.PyLong_FromLongLong(l)), + int i => PyObject.Create(CPythonAPI.PyLong_FromLong(i)), + bool b => PyObject.Create(CPythonAPI.PyBool_FromLong(b ? 1 : 0)), + double d => PyObject.Create(CPythonAPI.PyFloat_FromDouble(d)), IDictionary dictionary => ConvertFromDictionary(context, culture, dictionary), ITuple t => ConvertFromTuple(context, culture, t), IEnumerable e => ConvertFromList(context, culture, e), BigInteger b => ConvertFromBigInteger(context, culture, b), - PyObject pyObject => pyObject, + PyObject pyObject => pyObject.Clone(), null => PyObject.None, _ => base.ConvertFrom(context, culture, value) }; diff --git a/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs b/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs index d8d5abe0..c2f041ee 100644 --- a/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs +++ b/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs @@ -1,8 +1,12 @@ using CSnakes.Runtime.CPython; namespace CSnakes.Runtime.Python.Interns; -internal class ImmortalPyObject(nint handle) : PyObject(handle) +internal class ImmortalPyObject : PyObject { + internal ImmortalPyObject(nint handle) : base(handle) + { + } + protected override bool ReleaseHandle() { return true; diff --git a/src/CSnakes.Runtime/Python/PyObject.cs b/src/CSnakes.Runtime/Python/PyObject.cs index f21b031f..2f28d010 100644 --- a/src/CSnakes.Runtime/Python/PyObject.cs +++ b/src/CSnakes.Runtime/Python/PyObject.cs @@ -13,7 +13,7 @@ public class PyObject : SafeHandle private static readonly TypeConverter td = TypeDescriptor.GetConverter(typeof(PyObject)); internal static PyObject? none; - internal PyObject(IntPtr pyObject, bool ownsHandle = true) : base(pyObject, ownsHandle) + protected PyObject(IntPtr pyObject, bool ownsHandle = true) : base(pyObject, ownsHandle) { if (pyObject == IntPtr.Zero) { @@ -21,6 +21,15 @@ internal PyObject(IntPtr pyObject, bool ownsHandle = true) : base(pyObject, owns } } + internal static PyObject Create(IntPtr ptr) + { + if (none is null) + throw new InvalidOperationException("Python is not initialized. You cannot call this method outside of a Python Environment context."); + if (none.DangerousGetHandle() == ptr) + return none; + return new PyObject(ptr); + } + public override bool IsInvalid => handle == IntPtr.Zero; protected override bool ReleaseHandle() diff --git a/src/CSnakes.Runtime/Python/PyTuple.cs b/src/CSnakes.Runtime/Python/PyTuple.cs index 5d9fd633..c7484a47 100644 --- a/src/CSnakes.Runtime/Python/PyTuple.cs +++ b/src/CSnakes.Runtime/Python/PyTuple.cs @@ -18,7 +18,7 @@ public static PyObject CreateTuple(IEnumerable items) marshallers.Add(m); handles.Add(m.ToUnmanaged()); } - return new(CPythonAPI.PackTuple(handles.ToArray())); + return PyObject.Create(CPythonAPI.PackTuple(handles.ToArray())); } finally { From 9d15b278f312e413025592305da2e18d8a37a9ed Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Wed, 21 Aug 2024 14:54:02 +1000 Subject: [PATCH 06/11] Apply suggestions from code review Co-authored-by: Aaron Powell Co-authored-by: Aaron Robinson --- src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs | 6 ++---- src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs | 3 ++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs b/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs index d8d5abe0..4677e5de 100644 --- a/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs +++ b/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs @@ -3,10 +3,8 @@ namespace CSnakes.Runtime.Python.Interns; internal class ImmortalPyObject(nint handle) : PyObject(handle) { - protected override bool ReleaseHandle() - { - return true; - } + protected override bool ReleaseHandle() => true; + protected override void Dispose(bool disposing) { diff --git a/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs b/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs index c0b68c05..eb25b250 100644 --- a/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs +++ b/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs @@ -2,7 +2,8 @@ namespace CSnakes.Runtime.Python.Interns; -internal class PyNoneObject : ImmortalPyObject +internal sealed class PyNoneObject : ImmortalPyObject + { public PyNoneObject() : base(CPythonAPI.GetNone()) { From 51ad555dfffdb3faee14a5d1f16140ad778c90f6 Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Wed, 21 Aug 2024 14:55:00 +1000 Subject: [PATCH 07/11] Update src/CSnakes.Runtime/Python/PyObject.cs Co-authored-by: Aaron Powell --- src/CSnakes.Runtime/Python/PyObject.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/CSnakes.Runtime/Python/PyObject.cs b/src/CSnakes.Runtime/Python/PyObject.cs index f21b031f..ad3e4688 100644 --- a/src/CSnakes.Runtime/Python/PyObject.cs +++ b/src/CSnakes.Runtime/Python/PyObject.cs @@ -162,10 +162,8 @@ public virtual string GetRepr() /// Is the Python object None? /// /// true if None, else false - public virtual bool IsNone() - { - return CPythonAPI.IsNone(this); - } + public virtual bool IsNone() => CPythonAPI.IsNone(this); + public static PyObject None { From 9fdbfe563947b2e2d931263c42b103a46041945b Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Wed, 21 Aug 2024 14:55:12 +1000 Subject: [PATCH 08/11] Update src/CSnakes.Runtime/Python/PyObject.cs Co-authored-by: Aaron Robinson --- src/CSnakes.Runtime/Python/PyObject.cs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/CSnakes.Runtime/Python/PyObject.cs b/src/CSnakes.Runtime/Python/PyObject.cs index ad3e4688..e8ab6309 100644 --- a/src/CSnakes.Runtime/Python/PyObject.cs +++ b/src/CSnakes.Runtime/Python/PyObject.cs @@ -165,15 +165,7 @@ public virtual string GetRepr() public virtual bool IsNone() => CPythonAPI.IsNone(this); - public static PyObject None - { - get - { - if (none is null) - throw new InvalidOperationException("Python is not initialized. You cannot call this method outside of a Python Environment context."); - return none; - } - } + public static PyObject None { get; } = new PyNoneObject(); /// /// Call the object. Equivalent to (__call__)(args) From 0485a6711bdae56c61bf4d8ec9b0346d5e6768fb Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Wed, 21 Aug 2024 15:05:41 +1000 Subject: [PATCH 09/11] Move the static to avoid doing a runtime check on every Create call --- src/CSnakes.Runtime/CPython/Init.cs | 1 - src/CSnakes.Runtime/CPython/None.cs | 2 -- .../Python/Interns/PyNoneObject.cs | 6 ++++- src/CSnakes.Runtime/Python/PyObject.cs | 23 ++++++++----------- 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/CSnakes.Runtime/CPython/Init.cs b/src/CSnakes.Runtime/CPython/Init.cs index 8130bbed..fcabaa9f 100644 --- a/src/CSnakes.Runtime/CPython/Init.cs +++ b/src/CSnakes.Runtime/CPython/Init.cs @@ -84,7 +84,6 @@ private void InitializeEmbeddedPython() // Import builtins module var builtinsMod = Import("builtins"); PyNone = GetAttr(builtinsMod, "None"); - PyObject.none = new PyNoneObject(); Py_DecRef(builtinsMod); } PyEval_SaveThread(); diff --git a/src/CSnakes.Runtime/CPython/None.cs b/src/CSnakes.Runtime/CPython/None.cs index d285f369..a56ebac1 100644 --- a/src/CSnakes.Runtime/CPython/None.cs +++ b/src/CSnakes.Runtime/CPython/None.cs @@ -1,6 +1,4 @@ using CSnakes.Runtime.Python; -using System.ComponentModel; - namespace CSnakes.Runtime.CPython; internal unsafe partial class CPythonAPI diff --git a/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs b/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs index eb25b250..257b4670 100644 --- a/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs +++ b/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs @@ -3,7 +3,6 @@ namespace CSnakes.Runtime.Python.Interns; internal sealed class PyNoneObject : ImmortalPyObject - { public PyNoneObject() : base(CPythonAPI.GetNone()) { @@ -15,4 +14,9 @@ public PyNoneObject() : base(CPythonAPI.GetNone()) public override string ToString() => "None"; + internal override PyObject Clone() + { + return this; + } + } diff --git a/src/CSnakes.Runtime/Python/PyObject.cs b/src/CSnakes.Runtime/Python/PyObject.cs index 844fced7..1f5e26dc 100644 --- a/src/CSnakes.Runtime/Python/PyObject.cs +++ b/src/CSnakes.Runtime/Python/PyObject.cs @@ -1,4 +1,5 @@ using CSnakes.Runtime.CPython; +using CSnakes.Runtime.Python.Interns; using System.ComponentModel; using System.Diagnostics; using System.Runtime.InteropServices; @@ -11,7 +12,6 @@ namespace CSnakes.Runtime.Python; public class PyObject : SafeHandle { private static readonly TypeConverter td = TypeDescriptor.GetConverter(typeof(PyObject)); - internal static PyObject? none; protected PyObject(IntPtr pyObject, bool ownsHandle = true) : base(pyObject, ownsHandle) { @@ -23,10 +23,8 @@ protected PyObject(IntPtr pyObject, bool ownsHandle = true) : base(pyObject, own internal static PyObject Create(IntPtr ptr) { - if (none is null) - throw new InvalidOperationException("Python is not initialized. You cannot call this method outside of a Python Environment context."); - if (none.DangerousGetHandle() == ptr) - return none; + if (None.DangerousGetHandle() == ptr) + return None; return new PyObject(ptr); } @@ -73,13 +71,13 @@ internal static void ThrowPythonExceptionAsClrException() throw new InvalidDataException("An error occurred in Python, but no exception was set."); } - using var pyExceptionType = new PyObject(excType); + using var pyExceptionType = Create(excType); PyObject? pyExceptionTraceback = excTraceback == IntPtr.Zero ? null : new PyObject(excTraceback); var pyExceptionStr = string.Empty; if (excValue != IntPtr.Zero) { - using PyObject pyExceptionValue = new PyObject(excValue); + using PyObject pyExceptionValue = Create(excValue); pyExceptionStr = pyExceptionValue.ToString(); } ; @@ -126,7 +124,7 @@ public virtual PyObject GetAttr(string name) RaiseOnPythonNotInitialized(); using (GIL.Acquire()) { - return new PyObject(CPythonAPI.GetAttr(this, name)); + return Create(CPythonAPI.GetAttr(this, name)); } } @@ -148,7 +146,7 @@ public virtual PyObject GetIter() RaiseOnPythonNotInitialized(); using (GIL.Acquire()) { - return new PyObject(CPythonAPI.PyObject_GetIter(this)); + return Create(CPythonAPI.PyObject_GetIter(this)); } } @@ -173,7 +171,6 @@ public virtual string GetRepr() /// true if None, else false public virtual bool IsNone() => CPythonAPI.IsNone(this); - public static PyObject None { get; } = new PyNoneObject(); /// @@ -207,7 +204,7 @@ public PyObject CallWithArgs(PyObject[]? args = null) { using (GIL.Acquire()) { - return new PyObject(CPythonAPI.Call(this, argHandles)); + return Create(CPythonAPI.Call(this, argHandles)); } } finally { @@ -253,7 +250,7 @@ public PyObject CallWithKeywordArguments(PyObject[]? args = null, string[]? kwna { using (GIL.Acquire()) { - return new PyObject(CPythonAPI.Call(this, argHandles, kwnames, kwargHandles)); + return Create(CPythonAPI.Call(this, argHandles, kwnames, kwargHandles)); } } finally @@ -316,7 +313,7 @@ public T As() } } - internal PyObject Clone() + internal virtual PyObject Clone() { CPythonAPI.Py_IncRefRaw(handle); return new PyObject(handle); From ba7d20ef1b93da08623a49166df43ec4a1d991a7 Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Wed, 21 Aug 2024 15:17:00 +1000 Subject: [PATCH 10/11] Organize the builtins so that it doesn't need to be called after None is stored --- src/CSnakes.Runtime/CPython/Import.cs | 15 +++++++++++++++ src/CSnakes.Runtime/CPython/Init.cs | 6 +----- src/CSnakes.Runtime/CPython/Object.cs | 3 +++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/CSnakes.Runtime/CPython/Import.cs b/src/CSnakes.Runtime/CPython/Import.cs index bfca77ef..e8389179 100644 --- a/src/CSnakes.Runtime/CPython/Import.cs +++ b/src/CSnakes.Runtime/CPython/Import.cs @@ -18,6 +18,21 @@ internal static PyObject Import(string name) return PyObject.Create(module); } + protected static nint GetBuiltin(string name) + { + nint pyName = AsPyUnicodeObject("builtins"); + nint pyAttrName = AsPyUnicodeObject(name); + nint module = PyImport_Import(pyName); + nint attr = PyObject_GetAttrRaw(module, pyAttrName); + if (attr == IntPtr.Zero) + { + PyObject.ThrowPythonExceptionAsClrException(); + } + Py_DecRefRaw(pyName); + Py_DecRefRaw(pyAttrName); + return attr; + } + /// /// Import and return a reference to the module `name` /// diff --git a/src/CSnakes.Runtime/CPython/Init.cs b/src/CSnakes.Runtime/CPython/Init.cs index fcabaa9f..3119a98f 100644 --- a/src/CSnakes.Runtime/CPython/Init.cs +++ b/src/CSnakes.Runtime/CPython/Init.cs @@ -80,11 +80,7 @@ private void InitializeEmbeddedPython() PyDictType = GetTypeRaw(PyDict_New()); PyBytesType = GetTypeRaw(PyBytes_FromByteSpan(new byte[] { })); ItemsStrIntern = AsPyUnicodeObject("items"); - - // Import builtins module - var builtinsMod = Import("builtins"); - PyNone = GetAttr(builtinsMod, "None"); - Py_DecRef(builtinsMod); + PyNone = GetBuiltin("None"); } PyEval_SaveThread(); } diff --git a/src/CSnakes.Runtime/CPython/Object.cs b/src/CSnakes.Runtime/CPython/Object.cs index d4c59056..3fcc9a6e 100644 --- a/src/CSnakes.Runtime/CPython/Object.cs +++ b/src/CSnakes.Runtime/CPython/Object.cs @@ -106,6 +106,9 @@ internal static bool HasAttr(PyObject ob, string name) [LibraryImport(PythonLibraryName)] internal static partial IntPtr PyObject_GetAttr(PyObject ob, IntPtr attr); + [LibraryImport(PythonLibraryName, EntryPoint = "PyObject_GetAttr")] + private static partial IntPtr PyObject_GetAttrRaw(IntPtr ob, IntPtr attr); + /// /// Does the object ob have the attr `attr`? /// From 8517b1b7d4b6d96d673a4dde5e183e8348c9838d Mon Sep 17 00:00:00 2001 From: Anthony Shaw Date: Wed, 21 Aug 2024 15:19:47 +1000 Subject: [PATCH 11/11] Code cleanup --- src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs | 5 ++--- src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs | 6 +----- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs b/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs index d5b74503..013b9bfa 100644 --- a/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs +++ b/src/CSnakes.Runtime/Python/Interns/ImmortalPyObject.cs @@ -1,5 +1,4 @@ -using CSnakes.Runtime.CPython; -namespace CSnakes.Runtime.Python.Interns; +namespace CSnakes.Runtime.Python.Interns; internal class ImmortalPyObject : PyObject { @@ -10,7 +9,7 @@ internal ImmortalPyObject(nint handle) : base(handle) protected override bool ReleaseHandle() => true; - protected override void Dispose(bool disposing) + protected override void Dispose(bool disposing) { // I am immortal!! } diff --git a/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs b/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs index 257b4670..967e2d7a 100644 --- a/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs +++ b/src/CSnakes.Runtime/Python/Interns/PyNoneObject.cs @@ -14,9 +14,5 @@ public PyNoneObject() : base(CPythonAPI.GetNone()) public override string ToString() => "None"; - internal override PyObject Clone() - { - return this; - } - + internal override PyObject Clone() => this; }