- Notifications
You must be signed in to change notification settings - Fork749
Description
Environment
- Pythonnet version: 3.0.5
- Python version: 3.13
- Operating System: MacOS Sequoia 15.1.1
- .NET Runtime: 8.0.403
Details
After#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, ...).
Here are my results, along with a few proposed fixes. Please let me know if there are any mistakes.
Shared setup
importpythonnetpythonnet.load('coreclr')importclrfromSystem.Collections.GenericimportDictionary,List,KeyValuePairfromSystem.Collections.ImmutableimportImmutableArray,ImmutableList,ImmutableDictionaryfromSystemimportInt32,String,Object,Nullable,ArrayfromSystem.Collections.GenericimportCollectionExtensions
Array-like containers
ArrT=Array[Int32]ImmArrT=ImmutableArray[Int32]ListT=List[Int32]arr=ArrT([0,1,2,3,4])immarr=ImmutableArray.Create[Int32](0,1,2,3,4)# is this supposed to work, instead of using Create()?lst=ListT(ArrT([0,1,2,3,4]))immlst=ImmutableList.ToImmutableList[Int32](ArrT([0,1,2,3,4]))
What worked everywhere
__iter____reversed____contains____len__reverse()__getitem__when the index is within boundsindex()when the element is in the container__setitem__when the index is within bounds
What didn't work
__getitem__when the index is negative or out of bounds
arr[-1]# OKarr[999]# OK, raises IndexErrorimmarr[-1]# System.IndexOutOfRangeException: Index was outside the bounds of the array.lst[-1]# System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')immlst[-1]# System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values. (Parameter 'index')
Python expects a negative index to be interpreted as the position from the end of the container. Otherwise this breaks the implementation ofpop() from abc (on top of a common assumption). This translation is only done for Array (here) for some reason.
Also, the exception when the index is out of bounds should beIndexError, otherwise this breaks the implementation ofSequence.index() among other things.
Possible fix
Add the following method to classSequenceMixin:
def__getitem__(self,key):length=len(self)key=keyifkey>=0elselength+keyifkeynotinrange(length):raiseIndexError('index out of range')returnself.get_Item(key)
... and make sureList doesn't override it?
index()when the element is NOT in the container
arr.index(999)# OK, raises ValueErrorimmarr.index(999)# System.IndexOutOfRangeException: Index was outside the bounds of the array.lst.index(999)# System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')immlst.index(999)# System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values. (Parameter 'index')
This is caused by the previous point, the exceptions are thrown inSequence.index().
__getitem__,__setitem__and__delitem__with slices
arr[0:3]# TypeError: array index has type slice, expected an integerimmarr[0:3]# TypeError: No method matches given arguments for ImmutableArray`1.get_Item: (<class 'slice'>)lst[0:3]# TypeError: No method matches given arguments for List`1.get_Item: (<class 'slice'>)immlst[0:3]# TypeError: No method matches given arguments for ImmutableList`1.get_Item: (<class 'slice'>)
This is ok, clearly not supported which is fine. Exceptions self-explanatory.
__setitem__when the index is negative or out of bounds
arr[-1]=9# OKlst[-1]=9# System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')arr[999]=9# OK, raises IndexErrorlst[999]=9# System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')
OK forArray; forList, same as__getitem__: a negative index is not translated, and an out of bounds index raises the wrong exception.
Possible fix
Like for__getitem__, add the following method to classMutableSequenceMixin:
def__setitem__(self,key,value):length=len(self)key=keyifkey>=0elselength+keyifkeynotinrange(length):raiseIndexError('index out of range')self.set_Item(key,value)
- methods that would result in a different
Arraylength
delarr[4]# SystemError: Objects/longobject.c:583: bad argument to internal functionarr.insert(0,5)# IndexError; calls abc implementation, which is a stub!arr.append(5)# IndexError; uses abc implementation, which falls back to insertarr.pop()# SystemError (same as del); uses abc implementation, which calls delarr.clear()# SystemError (same as del); uses abc implementation, which calls delarr.extend([10,11,12])# IndexError; uses abc implementation, which calls appendarr.remove(9)# SystemError (same as del); uses abc implementation, which calls delarr+= [10,11,12]# IndexError; uses abc implementation, which falls back to extend
This could be fine, to disallow changing an array's length, even thoughArray.Resize andArray.Copy could in theory allow us to have all of these working and be fairly efficient.
But the exceptions have me think that this is another kind of problem:
- the
SystemErrorcomes from the bowels of CPython, preciselyhere, so I assume this is unintended behavior; - the
IndexErroris caused byMutableSequenceMixinnot implementinginsert(); if not supporting this is intended, it could be made more understandable by implementing it (inArray, notMutableSequenceMixin) and throwingTypeError,System.NotSupportedExceptionor something more explanatory.
- deleting
Listelements
dellst[0]# crash
Crash output
Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object. at Python.Runtime.BorrowedReference.DangerousGetAddress() in /home/benedikt/.cache/uv/sdists-v6/.tmpkW03e6/pythonnet-3.0.5/src/runtime/Native/BorrowedReference.cs:line 18 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 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 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[1] 37819 abort python3 delme_test_pythonnet.pyLike forDictionary,__delitem__ on aList causes a hard crash; possibly related to#2530.
- other methods mutating
Listlength
lst.insert(0,5)# IndexError; calls abc implementation, which is a stub!lst.append(5)# IndexError; uses abc implementation, which calls insertlst.pop()# IndexError; uses abc implementation, which calls __getitem__(-1) and dellst.clear()# IndexError; uses abc implementation, which calls poplst.extend([10,11,12])# IndexError; uses abc implementation, which calls appendlst.remove(3)# CRASH; uses abc implementation, which calls dellst+= [10,11,12]# IndexError; uses abc implementation, which falls back to extend
These should definitely work; it mostly boils down to the previous point, the missing support for negative indexes andinsert() not being implemented inMutableMappingMixin.
Possible fix
Add the following method to classMutableSequenceMixin:
definsert(self,index,value):self.Insert(index,value)
And also apply some of the previous proposed fixes, on top of fixingdel.
Dictionary-like containers
DictT=Dictionary[Int32,String]DictT2=Dictionary[String,Int32]DictT3=Dictionary[String,Nullable[Int32]]d=DictT()d[10]="10"d[20]="20"d[30]="30"d2=DictT2()d3=DictT3()rod=CollectionExtensions.AsReadOnly[Int32,String](DictT(d))# ReadOnlyDictionary<int, string>immd=ImmutableDictionary.ToImmutableDictionary[Int32,String](d)# ImmutableDictionary<int, string>
What worked everywhere
__contains____len____getitem__when the key is in the dictionary__setitem__keysvaluesitems, although maybe it could be achieved without copyinggetclearupdate
What didn't work
- Deleting elements
deld[""]# CRASH
This crashes with the following output; already tracked in#2530.
Crash output
Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object. at Python.Runtime.BorrowedReference.DangerousGetAddress() in /home/benedikt/.cache/uv/sdists-v6/.tmpkW03e6/pythonnet-3.0.5/src/runtime/Native/BorrowedReference.cs:line 18 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 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 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[1] 44289 abort python3 delme_test_pythonnet.py__iter__returnsKeyValuePairs instead of tuples
list(d)# gives a list of KeyValuePair
This contradicts the Mapping protocol which mandates that the mapping is an iterable of its keys, and breaks thepopitem() implementation fromcollections.abc. At least the latter should be fixed.
popitem
d.popitem()# TypeError: No method matches given arguments for Dictionary`2.get_Item: (<class 'System.Collections.Generic.KeyValuePair[Int32,String]'>)d2.popitem()# TypeError: No method matches given arguments for Dictionary`2.get_Item: (<class 'System.Collections.Generic.KeyValuePair[String,Int32]'>)d3.popitem()# TypeError: No method matches given arguments for Dictionary`2.get_Item: (<class 'System.Collections.Generic.KeyValuePair[String,Nullable[Int32]]'>)
This is caused by the previous point, the exceptions are thrown inMutableMapping.popitem() which expects the dictionary to be an iterable of its keys.
Possible fix
Add the following toMutableMappingMixin:
defpopitem(self):try:key=next(iter(self.Keys))exceptStopIteration:raiseKeyErrorfromNonevalue=self[key]delself[key]returnkey,value
... on top of fixingdel.
__getitem__when the key is not in the dictionary
d[40]# KeyNotFoundException: The given key '40' was not present in the dictionary.rod[40]# KeyNotFoundException: The given key '40' was not present in the dictionary.immd[40]# KeyNotFoundException: The given key '40' was not present in the dictionary.
The exception is not translated to aValueError, which might theoretically break some functions trying to catch it, but it doesn't seem as important here.
popandsetdefaultfor some types
d.pop(10)# OKd2.pop("10")# TypeError: No method matches given arguments for Dictionary`2.TryGetValue: (<class 'str'>, <class 'NoneType'>)d3.pop("10")# OKd.setdefault(10,"10")# OKd2.setdefault("10",10)# TypeError: No method matches given arguments for Dictionary`2.TryGetValue: (<class 'str'>, <class 'NoneType'>)d3.setdefault("10",10)# OKd3.setdefault("10")# Also OK
I believe this breaks only with non-nullable value types: the exception is called from theMutableMappingMixin implementation ofpop(), which callsself.TryGetValue(key, None)here.
Possible fix: callself.TryGetValue(key) instead? I believe this is supported since commitf69753c.