Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent type conversion rules for CLI arrays #1320

Closed
BCSharp opened this issue Feb 17, 2022 · 3 comments · Fixed by #1322
Closed

Inconsistent type conversion rules for CLI arrays #1320

BCSharp opened this issue Feb 17, 2022 · 3 comments · Fixed by #1322
Assignees

Comments

@BCSharp
Copy link
Member

BCSharp commented Feb 17, 2022

Description

When assigning integer values to an element of a multi-dimensional array, the customary type conversions are not performed. However, they are performed when the array is 1-dimensional.

Steps to Reproduce

IronPython 3.4.0a1 (3.4.0.0001)
[.NETCoreApp,Version=v6.0 on .NET 6.0.1 (64-bit)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import clr
>>> clr.AddReference("System.Numerics")
>>> from System import Array, Int32
>>> from System.Numerics import BigInteger
>>> rank1arrayOfInt32 = Array[Int32](3)        
>>> rank1arrayOfBigInteger = Array[BigInteger](3) 
>>> rank1arrayOfBigInteger[0] = 1  # assigning Int32 to an element of type BigInteger: OK, widening conversion
>>> print(rank1arrayOfBigInteger)
Array[long]((1, 0, 0))
>>> rank1arrayOfInt32[0] = 1 << 64 >> 64 # assigning BigInteger to an element of type Int32: OK, narrowing conversion
>>> print(rank1arrayOfInt32)
Array[int]((1, 0, 0))
>>> rank2arrayOfInt32 = Array[Int32](3, 2)
>>> rank2arrayOfBigInteger = Array[BigInteger](3, 2)
>>> rank2arrayOfBigInteger[0, 0] = 1   # assigning Int32 to an element of type BigInteger: not OK?
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Object cannot be stored in an array of this type.
>>> rank2arrayOfInt32[0, 0] = 1 << 64 >> 64   # assigning BigInteger to an element of type Int32: not OK?      
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Object cannot be stored in an array of this type.
>>> rank2arrayOfInt32[0, 0] = 1  # assignments that do not require conversion work OK
>>> rank2arrayOfInt32[0, 0]                     
1
>>> rank2arrayOfBigInteger[0, 0] = 1 << 64 >> 64
>>> rank2arrayOfBigInteger[0, 0]
1

Expected behavior:

Multi-dimensional arrays behave like one-dimensional arrays.

Related Case

Something similar (though not the same) is happening with arrays of Single and Double: assigning the other type to a 1-dimensional array works in both ways, but when 2-dimensional arrays are used, assigning a Double to an array of Single fails (but with a different message than for integer types), but assigning a Single to an array of Double works fine.

>>> from System import Array, Single, Double
>>> rank1arrayOfSingle = Array[Single](3)
>>> rank1arrayOfDouble = Array[Double](3)   
>>> rank1arrayOfSingle[0] = Double(1.2)   # narrowing conversion
>>> rank1arrayOfDouble[0] = Single(1.2)   # widening conversion
>>> rank2arrayOfSingle = Array[Single](3, 2)
>>> rank2arrayOfDouble = Array[Double](3, 2)
>>> rank2arrayOfSingle[0, 0] = Double(1.2)  # no narrowing conversion?
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Cannot widen from source type to target type either because the source type is a not a primitive type or the conversion cannot be accomplished.
>>> rank2arrayOfDouble[0, 0] = Single(1.2)  # widening conversion

I will look into this issue in the coming days because it is blocking #52.

@BCSharp BCSharp self-assigned this Feb 17, 2022
@slozier
Copy link
Contributor

slozier commented Feb 17, 2022

A quick look at ArrayOps.SetItem seems to show that the one-dimentional version passes the value through Converter.Convert whereas the multi-dim one doesn't so should be an easy fix.

Not sure I'm a big fan of the lossy Single to Double narrowing conversion but I guess it is what it is.

@BCSharp
Copy link
Member Author

BCSharp commented Feb 17, 2022

Not sure I'm a big fan of the lossy Single to Double narrowing conversion but I guess it is what it is.

I don't like it either, I think lossy conversions better be explicit. I was thinking of modifying the 1-dim Single array to behave like n-dim Single, but if it goes through Converter, it would have a wide impact. Perhaps better to leave it as it is. I will have to think about it and run some tests.

@BCSharp
Copy link
Member Author

BCSharp commented Feb 18, 2022

I am going to leave the case of Double to Single automatic conversion for now. There is more going on with the floating point conversions. For instance, assigning Double to element of Array[Int64] or Array[Int32] will succeed by truncating the argument, but calling a generic method with type argument Int32 fails (I assume it is deliberate, because there is an explicit test for that case), while it success (by truncating) if the type argument is any other integer type, like Int64.

Perhaps the existing automatic conversion scheme is due for a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants