Cleanup and comments

Added a few cleanups and comments.
pythonnet · rmadsen-ks · Apr 27, 2022 · Apr 28, 2022 · Apr 29, 2022 · May 3, 2022
commit bb988cd523278114370d7a36ed50fb3572f9c4c0
diff --git a/src/runtime/Resources/clr.py b/src/runtime/Resources/clr.py
@@ -49,7 +49,12 @@ def __set__(self, instance, value):
         if not self.fset:
             raise AttributeError("%s is read-only" % self.__name__)
         return self.fset.__get__(instance, None)(value)
+
+    # TODO: I am not sure this add_attribute is actually necessary.
     def add_attribute(self, *args, **kwargs):
+        """Adds an attribute to this class.
+        If the first argument is a tuple we assume it is a tuple containing everything to initialize the attribute.
+        Otherwise, the first arg should be a .net type implementing Attribute."""
         lst = []
         if len(args) > 0:
             if isinstance(args[0], tuple):
@@ -60,7 +65,17 @@ def add_attribute(self, *args, **kwargs):
         return self
 
 class property(object):
+    """
+    property constructor for creating properties with implicit get/set.
+
+    It can be used as such:
+    e.g.::
+
+        class X(object):
+            A = clr.property(Double, 3.14)\
+               .add_attribute(Browsable(False))
 
+    """
     def __init__(self, type, default = None):
         import weakref
         self._clr_property_type_ = type
@@ -79,7 +94,11 @@ def __set__(self, instance, value):
             self.fset(instance,value)
             return
         self.values[instance] = value
+
     def add_attribute(self, *args, **kwargs):
+        """Adds an attribute to this class.
+        If the first argument is a tuple we assume it is a tuple containing everything to initialize the attribute.
+        Otherwise, the first arg should be a .net type implementing Attribute."""
         lst = []
         if len(args) > 0:
             if isinstance(args[0], tuple):
@@ -138,6 +157,9 @@ def __get__(self, instance, owner):
         return self.__func.__get__(instance, owner)
 
     def add_attribute(self, *args, **kwargs):
+        """Adds an attribute to this class.
+        If the first argument is a tuple we assume it is a tuple containing everything to initialize the attribute.
+        Otherwise, the first arg should be a .net type implementing Attribute."""
         lst = []
         if len(args) > 0:
             if isinstance(args[0], tuple):
@@ -148,7 +170,14 @@ def add_attribute(self, *args, **kwargs):
         return self
 
 class attribute(object):
+    """
+    Class decorator for adding attributes to .net python classes.
 
+    e.g.::
+        @attribute(DisplayName("X Class"))
+        class X(object):
+            pass
+    """
     def __init__(self, *args, **kwargs):
         lst = []
         if len(args) > 0:

diff --git a/src/runtime/Types/ClassDerived.cs b/src/runtime/Types/ClassDerived.cs
@@ -276,7 +276,10 @@ internal static Type CreateDerivedType(string name,
 
             Type baseClass = baseType;
             var interfaces = new HashSet<Type> { typeof(IPythonDerivedType) };
-            foreach(var t in interfaces2) interfaces.Add(t);
+
+            foreach(var interfaceType in interfaces2)
+                interfaces.Add(interfaceType);
+
             // if the base type is an interface then use System.Object as the base class
             // and add the base type to the list of interfaces this new class will implement.
             if (baseType.IsInterface)
@@ -285,6 +288,7 @@ internal static Type CreateDerivedType(string name,
                 baseClass = typeof(object);
             }
 
+            // __clr_abstract__ is used to create an abstract class.
             bool isAbstract = false;
             if (py_dict != null && Runtime.PyDict_Check(py_dict))
             {
@@ -298,10 +302,11 @@ internal static Type CreateDerivedType(string name,
                 baseClass,
                 interfaces.ToArray());
 
-            // add a field for storing the python object pointer
-            // FIXME: fb not used
+            // add a field for storing the python object pointer,
+            // but only if the baseclass does not already have it.
             if (baseClass.GetField(PyObjName, PyObjFlags) == null)
             {
+                // FIXME: fb not used
                 FieldBuilder fb = typeBuilder.DefineField(PyObjName,
 #pragma warning disable CS0618 // Type or member is obsolete. OK for internal use.
                     typeof(UnsafeReferenceWithRun),
@@ -373,9 +378,10 @@ internal static Type CreateDerivedType(string name,
             {
                 using var dict = new PyDict(py_dict);
 
+                // if there are any attributes, add them.
                 if (dict.HasKey("__clr_attributes__"))
                 {
-                    var attributes = new PyList(dict["__clr_attributes__"]);
+                    using var attributes = new PyList(dict["__clr_attributes__"]);
                     foreach (var attr in attributes)
                     {
                         var builder = AddAttribute(attr);
@@ -442,12 +448,10 @@ internal static Type CreateDerivedType(string name,
             return type;
         }
 
-        private static Dictionary<Type, PyType> pyTypeLookup = new Dictionary<Type, PyType>();
-
         /// <summary>
         /// Add a constructor override that calls the python ctor after calling the base type constructor.
         /// </summary>
-        /// <param name="ctor">constructor to be called before calling the python ctor</param>
+        /// <param name="ctor">constructor to be called before calling the python ctor. This can be null if there is no constructor. </param>
         /// <param name="baseType">Python callable object</param>
         /// <param name="typeBuilder">TypeBuilder for the new type the ctor is to be added to</param>
         private static void AddConstructor(ConstructorInfo ctor, Type baseType, TypeBuilder typeBuilder)
@@ -665,7 +669,7 @@ private static void AddPythonMethod(string methodName, PyObject func, TypeBuilde
                     methodBuilder.SetCustomAttribute(builder);
                 }
 
-                if (methodAssoc.TryGetValue(func, out var lst))
+                if (MethodAttributeAssociations.TryGetValue(func, out var lst))
                 {
                     foreach(var attr in lst)
                     {
@@ -674,7 +678,7 @@ private static void AddPythonMethod(string methodName, PyObject func, TypeBuilde
                         methodBuilder.SetCustomAttribute(builder);
                     }
 
-                    methodAssoc.Remove(func);
+                    MethodAttributeAssociations.Remove(func);
 
                 }
 
@@ -770,11 +774,12 @@ private static void AddPythonProperty(string propertyName, PyObject func, TypeBu
             var propertyType = result as Type;
             string pyTypeName = null;
             string pyTypeModule = null;
+            // if the property type is null, we assume that it is a python type
+            // and not a C# type, in this case the property is just a PyObject type instead.
             if (propertyType == null)
             {
                 propertyType = typeof(PyObject);
                 pyTypeModule = pyNativeType.GetAttr("__module__").ToString();
-                //throw new ArgumentException("_clr_property_type must be a CLR type");
                 pyTypeName = pyNativeType.Name;
             }
 
@@ -807,7 +812,7 @@ private static void AddPythonProperty(string propertyName, PyObject func, TypeBu
                 foreach (var fname in new[]{ "fget", "fset" })
                 {
                     using var pyfget = func.GetAttr(fname);
-                    if (methodAssoc.TryGetValue(pyfget, out var lst))
+                    if (MethodAttributeAssociations.TryGetValue(pyfget, out var lst))
                     {
                         foreach (var attr in lst)
                         {
@@ -816,7 +821,7 @@ private static void AddPythonProperty(string propertyName, PyObject func, TypeBu
                             propertyBuilder.SetCustomAttribute(builder);
                         }
 
-                        methodAssoc.Remove(func);
+                        MethodAttributeAssociations.Remove(func);
                     }
                 }
             }
@@ -915,28 +920,45 @@ private static ModuleBuilder GetModuleBuilder(string assemblyName, string module
     [Obsolete(Util.InternalUseOnly)]
     public class PythonDerivedType
     {
-        private static List<PyTuple> attributesStack = new List<PyTuple>();
-        internal static Dictionary<PyObject, List<PyTuple>> methodAssoc = new Dictionary<PyObject, List<PyTuple>>();
-        public static void PushAttribute(PyObject obj)
+        /// <summary> Tracks the attributes pushed with PushAttributes. </summary>
+        static List<PyTuple> attributesStack = new ();
+        /// <summary>
+        /// This field track associations between python functions and associated attributes.
+        /// </summary>
+        internal static Dictionary<PyObject, List<PyTuple>> MethodAttributeAssociations = new ();
+
+        /// <summary>
+        /// This pushes an attribute on the current attribute stack.
+        /// This happens when invoking e.g `@(attribute(Browsable(False))`.
+        /// </summary>
+        /// <param name="attribute">The pushed attribute.
+        /// This is should not be an attribute instance, but a tuple from which it can be created.</param>
+        public static void PushAttribute(PyObject attribute)
         {
             using var _ = Py.GIL();
-            var tp = new PyTuple(obj);
+            var tp = new PyTuple(attribute);
             attributesStack.Add(tp);
         }
 
-        public static bool AssocAttribute(PyObject obj, PyObject func)
+        /// <summary>
+        /// Associates an attribute with a function.
+        /// </summary>
+        /// <param name="attribute"></param>
+        /// <param name="func"></param>
+        /// <returns></returns>
+
+        public static bool AssocAttribute(PyObject attribute, PyObject func)
         {
             using var _ = Py.GIL();
-            var tp = new PyTuple(obj);
+            var tp = new PyTuple(attribute);
             for (int i = 0; i < attributesStack.Count; i++)
             {
-
-                if (tp.BorrowNullable()== attributesStack[i].BorrowNullable())
+                if (tp.BorrowNullable() == attributesStack[i].BorrowNullable())
                 {
                     attributesStack.RemoveAt(i);
-                    if (!methodAssoc.TryGetValue(func, out var lst))
+                    if (!MethodAttributeAssociations.TryGetValue(func, out var lst))
                     {
-                        lst = methodAssoc[func] = new List<PyTuple>();
+                        lst = MethodAttributeAssociations[func] = new List<PyTuple>();
                     }
                     lst.Add(tp);
                     return true;
@@ -947,13 +969,18 @@ public static bool AssocAttribute(PyObject obj, PyObject func)
         }
 
 
+        /// <summary>
+        /// Pops the current attribute stack and returns it. Any future pushed attributes will be in a new list.
+        /// </summary>
+        /// <returns></returns>
         public static IEnumerable<PyObject> PopAttributes()
         {
             if (attributesStack.Count == 0) return Array.Empty<PyObject>();
             var attrs = attributesStack;
             attributesStack = new List<PyTuple>();
             return attrs;
         }
+
         internal const string PyObjName = "__pyobj__";
         internal const BindingFlags PyObjFlags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.FlattenHierarchy;
 

diff --git a/src/runtime/Types/MetaType.cs b/src/runtime/Types/MetaType.cs
@@ -82,12 +82,7 @@ public static NewReference tp_new(BorrowedReference tp, BorrowedReference args,
             BorrowedReference bases = Runtime.PyTuple_GetItem(args, 1);
             BorrowedReference dict = Runtime.PyTuple_GetItem(args, 2);
 
-            // We do not support multiple inheritance, so the bases argument
-            // should be a 1-item tuple containing the type we are subtyping.
-            // That type must itself have a managed implementation. We check
-            // that by making sure its metatype is the CLR metatype.
-
-
+            // Extract interface types and base class types.
             List<Type> interfaces = new List<Type>();
             List<ClassBase> baseType = new List<ClassBase>();
 
@@ -105,25 +100,18 @@ public static NewReference tp_new(BorrowedReference tp, BorrowedReference args,
                 }
             }
 
+            // if the base type count is 0, there might still be interfaces to implement.
             if (baseType.Count == 0)
             {
                 baseType.Add(new ClassBase(typeof(object)));
             }
 
-
+            // Multiple inheritance is not supported, unless the other types are interfaces
             if (baseType.Count > 1)
             {
                 return Exceptions.RaiseTypeError("cannot use multiple inheritance with managed classes");
             }
 
-            /*
-            BorrowedReference mt = Runtime.PyObject_TYPE(baseType);
-
-            if (!(mt == PyCLRMetaType || mt == Runtime.PyTypeType))
-            {
-                return Exceptions.RaiseTypeError("invalid metatype");
-            }*/
-
             // Ensure that the reflected type is appropriate for subclassing,
             // disallowing subclassing of delegates, enums and array types.
 
@@ -146,7 +134,8 @@ public static NewReference tp_new(BorrowedReference tp, BorrowedReference args,
                 return Exceptions.RaiseTypeError("subclasses of managed classes do not support __slots__");
             }
 
-            // If __assembly__ or __namespace__ are in the class dictionary then create
+            // If the base class has a parameterless constructor, or
+            // if __assembly__ or __namespace__ are in the class dictionary then create
             // a managed sub type.
             // This creates a new managed type that can be used from .net to call back
             // into python.
@@ -156,7 +145,7 @@ public static NewReference tp_new(BorrowedReference tp, BorrowedReference args,
                 var ctor = btt?.GetConstructors(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)
                     .FirstOrDefault(x => x.GetParameters().Any() == false);
                 using var clsDict = new PyDict(dict);
-                
+
                 if (clsDict.HasKey("__assembly__") || clsDict.HasKey("__namespace__")
                                                    || (ctor != null))
                 {
@@ -167,7 +156,6 @@ public static NewReference tp_new(BorrowedReference tp, BorrowedReference args,
                     }
                     return TypeManager.CreateSubType(name, baseType, interfaces, clsDict);
                 }
-
             }
 
             var base_type = Runtime.PyTuple_GetItem(bases, 0);

diff --git a/src/runtime/Types/ModuleObject.cs b/src/runtime/Types/ModuleObject.cs
@@ -132,8 +132,10 @@ public NewReference GetAttribute(string name, bool guess)
                 return new NewReference(c);
             }
 
-            // attribute names
+            // Simplified imported attribute names
             // for attributes without the Attribute suffix, create an attribute builder.
+            // This means that imported attributes can be invoked using just e.g 'Browsable(False)'
+            // and not BrowsableAttribute(False) although its still an option.
             var qname2 = qname + "Attribute";
             var type2 = AssemblyManager.LookupTypes(qname2).FirstOrDefault(t => t.IsPublic);
             if (type2 != null)