René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Unimplemented or broken methods in Python wrappers to standard C# containers","articleBody":"### Environment\r\n\r\n-   Pythonnet version: 3.0.5\r\n-   Python version: 3.13\r\n-   Operating System: MacOS Sequoia 15.1.1\r\n-   .NET Runtime: 8.0.403\r\n\r\n### Details\r\n\r\nAfter #2530, I took the time to test from Python some of the most common C# containers, testing in particular the methods from the standard Python abc's (`Sequence`, `MutableSequence`, `Mapping`, `MutableMapping`, ...).\r\nHere are my results, along with a few proposed fixes. Please let me know if there are any mistakes.\r\n\r\n### Shared setup\r\n```python\r\nimport pythonnet\r\npythonnet.load('coreclr')\r\nimport clr\r\nfrom System.Collections.Generic import Dictionary, List, KeyValuePair\r\nfrom System.Collections.Immutable import ImmutableArray, ImmutableList, ImmutableDictionary\r\nfrom System import Int32, String, Object, Nullable, Array\r\nfrom System.Collections.Generic import CollectionExtensions\r\n```\r\n\r\n## Array-like containers\r\n```python\r\nArrT = Array[Int32]\r\nImmArrT = ImmutableArray[Int32]\r\nListT = List[Int32]\r\n\r\narr = ArrT([0, 1, 2, 3, 4])\r\nimmarr = ImmutableArray.Create[Int32](0, 1, 2, 3, 4)  # is this supposed to work, instead of using Create()?\r\nlst = ListT(ArrT([0, 1, 2, 3, 4]))\r\nimmlst = ImmutableList.ToImmutableList[Int32](ArrT([0, 1, 2, 3, 4]))\r\n```\r\n\r\n### What worked everywhere\r\n- `__iter__`\r\n- `__reversed__`\r\n- `__contains__`\r\n- `__len__`\r\n- `reverse()`\r\n- `__getitem__` when the index is within bounds\r\n- `index()` when the element is in the container\r\n- `__setitem__` when the index is within bounds\r\n\r\n### What didn't work\r\n- `__getitem__` when the index is negative or out of bounds\r\n```python\r\narr[-1]     # OK\r\narr[999]    # OK, raises IndexError\r\nimmarr[-1]  # System.IndexOutOfRangeException: Index was outside the bounds of the array.\r\nlst[-1]     # System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')\r\nimmlst[-1]  # System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values. (Parameter 'index')\r\n```\r\nPython expects a negative index to be interpreted as the position from the end of the container. Otherwise this breaks the implementation of `pop()` from abc (on top of a common assumption). This translation is only done for Array ([here](https://github.com/pythonnet/pythonnet/blob/dfd746b339dc24d530f7d64768e1c8acf9d84cb4/src/runtime/Types/ArrayObject.cs#L181-L184)) for some reason.\r\nAlso, the exception when the index is out of bounds should be `IndexError`, otherwise this breaks the implementation of `Sequence.index()` among other things.\r\n\r\n\u003cdetails\u003e\u003csummary\u003ePossible fix\u003c/summary\u003e\r\n\r\nAdd the following method to class `SequenceMixin`:\r\n```python\r\n    def __getitem__(self, key):\r\n        length = len(self)\r\n        key = key if key \u003e= 0 else length + key\r\n        if key not in range(length):\r\n            raise IndexError('index out of range')\r\n        return self.get_Item(key)\r\n```\r\n... and make sure `List` doesn't override it?\r\n\u003c/details\u003e \r\n\r\n- `index()` when the element is NOT in the container\r\n```python\r\narr.index(999)     # OK, raises ValueError\r\nimmarr.index(999)  # System.IndexOutOfRangeException: Index was outside the bounds of the array.\r\nlst.index(999)     # System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')\r\nimmlst.index(999)  # System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values. (Parameter 'index')\r\n```\r\nThis is caused by the previous point, the exceptions are thrown in `Sequence.index()`.\r\n\r\n- `__getitem__`, `__setitem__` and `__delitem__` with slices\r\n```python\r\narr[0:3]     # TypeError: array index has type slice, expected an integer\r\nimmarr[0:3]  # TypeError: No method matches given arguments for ImmutableArray`1.get_Item: (\u003cclass 'slice'\u003e)\r\nlst[0:3]     # TypeError: No method matches given arguments for List`1.get_Item: (\u003cclass 'slice'\u003e)\r\nimmlst[0:3]  # TypeError: No method matches given arguments for ImmutableList`1.get_Item: (\u003cclass 'slice'\u003e)\r\n```\r\nThis is ok, clearly not supported which is fine. Exceptions self-explanatory.\r\n\r\n- `__setitem__` when the index is negative or out of bounds\r\n```python\r\narr[-1] = 9   # OK\r\nlst[-1] = 9   # System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')\r\narr[999] = 9  # OK, raises IndexError\r\nlst[999] = 9  # System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')\r\n```\r\nOK for `Array`; for `List`, same as `__getitem__`: a negative index is not translated, and an out of bounds index raises the wrong exception.\r\n\r\n\u003cdetails\u003e\u003csummary\u003ePossible fix\u003c/summary\u003e\r\n\r\nLike for `__getitem__`, add the following method to class `MutableSequenceMixin`:\r\n```python\r\n    def __setitem__(self, key, value):\r\n        length = len(self)\r\n        key = key if key \u003e= 0 else length + key\r\n        if key not in range(length):\r\n            raise IndexError('index out of range')\r\n       self.set_Item(key, value)\r\n```\r\n\u003c/details\u003e \r\n\r\n- methods that would result in a different `Array` length\r\n```python\r\ndel arr[4]                # SystemError: Objects/longobject.c:583: bad argument to internal function\r\narr.insert(0, 5)          # IndexError; calls abc implementation, which is a stub!\r\narr.append(5)             # IndexError; uses abc implementation, which falls back to insert\r\narr.pop()                 # SystemError (same as del); uses abc implementation, which calls del\r\narr.clear()               # SystemError (same as del); uses abc implementation, which calls del\r\narr.extend([10, 11, 12])  # IndexError; uses abc implementation, which calls append\r\narr.remove(9)             # SystemError (same as del); uses abc implementation, which calls del\r\narr += [10, 11, 12]       # IndexError; uses abc implementation, which falls back to extend\r\n```\r\nThis could be fine, to disallow changing an array's length, even though `Array.Resize` and `Array.Copy` could in theory allow us to have all of these working and be fairly efficient.\r\nBut the exceptions have me think that this is another kind of problem:\r\n1. the `SystemError` comes from the bowels of CPython, precisely [here](https://github.com/python/cpython/blob/89f4b23f9efa4919e59cc542fd44062efc9898f2/Objects/longobject.c#L583), so I assume this is unintended behavior;\r\n2. the `IndexError` is caused by `MutableSequenceMixin` not implementing `insert()`; if not supporting this is intended, it could be made more understandable by implementing it (in `Array`, not `MutableSequenceMixin`) and throwing `TypeError`, `System.NotSupportedException` or something more explanatory.\r\n\r\n- deleting `List` elements\r\n```python\r\ndel lst[0]  # crash\r\n```\r\n\u003cdetails\u003e\u003csummary\u003eCrash output\u003c/summary\u003e\r\n\r\n```\r\nUnhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.\r\n   at Python.Runtime.BorrowedReference.DangerousGetAddress() in /home/benedikt/.cache/uv/sdists-v6/.tmpkW03e6/pythonnet-3.0.5/src/runtime/Native/BorrowedReference.cs:line 18\r\n   at Python.Runtime.NewReference..ctor(BorrowedReference reference, Boolean canBeNull) in /home/benedikt/.cache/uv/sdists-v6/.tmpkW03e6/pythonnet-3.0.5/src/runtime/Native/NewReference.cs:line 20\r\n   at Python.Runtime.Runtime.PyTuple_SetItem(BorrowedReference pointer, IntPtr index, BorrowedReference value) in /home/benedikt/.cache/uv/sdists-v6/.tmpkW03e6/pythonnet-3.0.5/src/runtime/Runtime.cs:line 1482\r\n   at Python.Runtime.ClassBase.mp_ass_subscript_impl(BorrowedReference ob, BorrowedReference idx, BorrowedReference v) in /home/benedikt/.cache/uv/sdists-v6/.tmpkW03e6/pythonnet-3.0.5/src/runtime/Types/ClassBase.cs:line 498\r\n[1]    37819 abort      python3 delme_test_pythonnet.py\r\n```\r\n\u003c/details\u003e\r\n\r\nLike for `Dictionary`, `__delitem__` on a `List` causes a hard crash; possibly related to #2530.\r\n\r\n- other methods mutating `List` length\r\n```python\r\nlst.insert(0, 5)          # IndexError; calls abc implementation, which is a stub!\r\nlst.append(5)             # IndexError; uses abc implementation, which calls insert\r\nlst.pop()                 # IndexError; uses abc implementation, which calls __getitem__(-1) and del\r\nlst.clear()               # IndexError; uses abc implementation, which calls pop\r\nlst.extend([10, 11, 12])  # IndexError; uses abc implementation, which calls append\r\nlst.remove(3)             # CRASH; uses abc implementation, which calls del\r\nlst += [10, 11, 12]       # IndexError; uses abc implementation, which falls back to extend\r\n```\r\nThese should definitely work; it mostly boils down to the previous point, the missing support for negative indexes and `insert()` not being implemented in `MutableMappingMixin`.\r\n\r\n\u003cdetails\u003e\u003csummary\u003ePossible fix\u003c/summary\u003e\r\n\r\nAdd the following method to class `MutableSequenceMixin`:\r\n```python\r\n    def insert(self, index, value):\r\n        self.Insert(index, value)\r\n```\r\nAnd also apply some of the previous proposed fixes, on top of fixing `del`.\r\n\u003c/details\u003e \r\n\r\n\r\n## Dictionary-like containers\r\n```python\r\nDictT  = Dictionary[Int32, String]\r\nDictT2 = Dictionary[String, Int32]\r\nDictT3 = Dictionary[String, Nullable[Int32]]\r\n\r\nd = DictT()\r\nd[10] = \"10\"\r\nd[20] = \"20\"\r\nd[30] = \"30\"\r\nd2 = DictT2()\r\nd3 = DictT3()\r\nrod = CollectionExtensions.AsReadOnly[Int32, String](DictT(d))  # ReadOnlyDictionary\u003cint, string\u003e\r\nimmd = ImmutableDictionary.ToImmutableDictionary[Int32, String](d)  # ImmutableDictionary\u003cint, string\u003e\r\n```\r\n\r\n### What worked everywhere\r\n- `__contains__`\r\n- `__len__`\r\n- `__getitem__` when the key is in the dictionary\r\n- `__setitem__`\r\n- `keys`\r\n- `values`\r\n- `items`, although maybe it could be achieved without copying\r\n- `get`\r\n- `clear`\r\n- `update`\r\n\r\n### What didn't work\r\n- Deleting elements\r\n```python\r\ndel d[\"\"]  # CRASH\r\n```\r\nThis crashes with the following output; already tracked in #2530.\r\n\r\n\u003cdetails\u003e\u003csummary\u003eCrash output\u003c/summary\u003e\r\n\r\n```\r\nUnhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.\r\n   at Python.Runtime.BorrowedReference.DangerousGetAddress() in /home/benedikt/.cache/uv/sdists-v6/.tmpkW03e6/pythonnet-3.0.5/src/runtime/Native/BorrowedReference.cs:line 18\r\n   at Python.Runtime.NewReference..ctor(BorrowedReference reference, Boolean canBeNull) in /home/benedikt/.cache/uv/sdists-v6/.tmpkW03e6/pythonnet-3.0.5/src/runtime/Native/NewReference.cs:line 20\r\n   at Python.Runtime.Runtime.PyTuple_SetItem(BorrowedReference pointer, IntPtr index, BorrowedReference value) in /home/benedikt/.cache/uv/sdists-v6/.tmpkW03e6/pythonnet-3.0.5/src/runtime/Runtime.cs:line 1482\r\n   at Python.Runtime.ClassBase.mp_ass_subscript_impl(BorrowedReference ob, BorrowedReference idx, BorrowedReference v) in /home/benedikt/.cache/uv/sdists-v6/.tmpkW03e6/pythonnet-3.0.5/src/runtime/Types/ClassBase.cs:line 498\r\n[1]    44289 abort      python3 delme_test_pythonnet.py\r\n```\r\n\u003c/details\u003e\r\n\r\n- `__iter__` returns `KeyValuePair`s instead of tuples\r\n```python\r\nlist(d)  # gives a list of KeyValuePair\r\n```\r\nThis contradicts the Mapping protocol which mandates that the mapping is an iterable of its keys, and breaks the `popitem()` implementation from `collections.abc`. At least the latter should be fixed.\r\n\r\n- `popitem`\r\n```python\r\nd.popitem()   # TypeError: No method matches given arguments for Dictionary`2.get_Item: (\u003cclass 'System.Collections.Generic.KeyValuePair[Int32,String]'\u003e)\r\nd2.popitem()  # TypeError: No method matches given arguments for Dictionary`2.get_Item: (\u003cclass 'System.Collections.Generic.KeyValuePair[String,Int32]'\u003e)\r\nd3.popitem()  # TypeError: No method matches given arguments for Dictionary`2.get_Item: (\u003cclass 'System.Collections.Generic.KeyValuePair[String,Nullable[Int32]]'\u003e)\r\n```\r\nThis is caused by the previous point, the exceptions are thrown in `MutableMapping.popitem()` which expects the dictionary to be an iterable of its keys.\r\n\r\n\u003cdetails\u003e\u003csummary\u003ePossible fix\u003c/summary\u003e\r\n\r\nAdd the following to `MutableMappingMixin`:\r\n```python\r\n   def popitem(self):\r\n       try:\r\n           key = next(iter(self.Keys))\r\n       except StopIteration:\r\n           raise KeyError from None\r\n       value = self[key]\r\n       del self[key]\r\n       return key, value\r\n```\r\n... on top of fixing `del`.\r\n\u003c/details\u003e\r\n\r\n\r\n- `__getitem__` when the key is not in the dictionary\r\n```python\r\nd[40]     # KeyNotFoundException: The given key '40' was not present in the dictionary.\r\nrod[40]   # KeyNotFoundException: The given key '40' was not present in the dictionary.\r\nimmd[40]  # KeyNotFoundException: The given key '40' was not present in the dictionary.\r\n```\r\nThe exception is not translated to a `ValueError`, which might theoretically break some functions trying to catch it, but it doesn't seem as important here.\r\n\r\n- `pop` and `setdefault` for some types\r\n```python\r\nd.pop(10)   # OK\r\nd2.pop(\"10\")  # TypeError: No method matches given arguments for Dictionary`2.TryGetValue: (\u003cclass 'str'\u003e, \u003cclass 'NoneType'\u003e)\r\nd3.pop(\"10\")  # OK\r\nd.setdefault(10, \"10\")   # OK\r\nd2.setdefault(\"10\", 10)  # TypeError: No method matches given arguments for Dictionary`2.TryGetValue: (\u003cclass 'str'\u003e, \u003cclass 'NoneType'\u003e)\r\nd3.setdefault(\"10\", 10)  # OK\r\nd3.setdefault(\"10\")      # Also OK\r\n```\r\nI believe this breaks only with non-nullable value types: the exception is called from the `MutableMappingMixin` implementation of `pop()`, which calls `self.TryGetValue(key, None)` [here](https://github.com/pythonnet/pythonnet/blob/dfd746b339dc24d530f7d64768e1c8acf9d84cb4/src/runtime/Mixins/collections.py#L65).\r\n**Possible fix**: call `self.TryGetValue(key)` instead? I believe this is supported since commit f69753c.\r\n\r\n","author":{"url":"https://github.com/DiegoBaldassarMilleuno","@type":"Person","name":"DiegoBaldassarMilleuno"},"datePublished":"2024-12-17T11:57:22.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":3},"url":"https://github.com/2531/pythonnet/issues/2531"}