Skip to content

Implement System.Array constructor with base arg #1828

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

Merged
merged 1 commit into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions Src/IronPython/Runtime/Operations/ArrayOps.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,23 @@ public static object __new__(CodeContext context, PythonType pythonType, ICollec
}

[StaticExtensionMethod]
public static object __new__(CodeContext context, PythonType pythonType, object items) {
public static object __new__(CodeContext context, PythonType pythonType, object items)
=> __new__(context, pythonType, items, @base: 0);

[StaticExtensionMethod]
public static object __new__(CodeContext context, PythonType pythonType, object items, /*[KeywordOnly]*/ int @base) {
Type type = pythonType.UnderlyingSystemType.GetElementType()!;

object? lenFunc;
if (!PythonOps.TryGetBoundAttr(items, "__len__", out lenFunc))
if (!PythonOps.TryGetBoundAttr(items, "__len__", out object? lenFunc))
throw PythonOps.TypeErrorForBadInstance("expected object with __len__ function, got {0}", items);

int len = context.LanguageContext.ConvertToInt32(PythonOps.CallWithContext(context, lenFunc));

Array res = Array.CreateInstance(type, len);
Array res = @base == 0 ?
Array.CreateInstance(type, len) : Array.CreateInstance(type, [len], [@base]);

IEnumerator ie = PythonOps.GetEnumerator(items);
int i = 0;
int i = @base;
while (ie.MoveNext()) {
res.SetValue(Converter.Convert(ie.Current, type), i++);
}
Expand Down Expand Up @@ -277,7 +281,7 @@ public static string __repr__(CodeContext/*!*/ context, [NotNone] Array/*!*/ sel
}
ret.Append(')');
if (self.GetLowerBound(0) != 0) {
ret.Append(", base: ");
ret.Append(", base=");
ret.Append(self.GetLowerBound(0));
}
ret.Append(')');
Expand Down
26 changes: 22 additions & 4 deletions Tests/test_array.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,26 @@ def test_constructor(self):
for y in range(array3.GetLength(1)):
self.assertEqual(array3[x, y], 0)

def test_constructor_nonzero_lowerbound(self):
# 1-based
arr = System.Array[int]((1, 2), base=1)
self.assertEqual(arr.Rank, 1)
self.assertEqual(arr.Length, 2)
self.assertEqual(arr.GetLowerBound(0), 1)
self.assertEqual(arr.GetUpperBound(0), 2)
self.assertEqual(arr[1], 1)
self.assertEqual(arr[2], 2)
for i in range(1, 3):
self.assertEqual(arr[i], i)

def test_repr(self):
from System import Array
arr = Array[int]((5, 1), base=1)
s = repr(arr)
self.assertEqual(s, "Array[int]((5, 1), base=1)")
array4eval = eval(s, globals(), locals())
self.assertEqual(arr, array4eval)

def test_nonzero_lowerbound(self):
a = System.Array.CreateInstance(int, (5,), (5,))
for i in range(5, 5 + a.Length): a[i] = i
Expand All @@ -208,7 +228,7 @@ def test_nonzero_lowerbound(self):
self.assertEqual(a[-1:-3:-1], System.Array[int]((9,8)))
self.assertEqual(a[-1], 9)

self.assertEqual(repr(a), 'Array[int]((5, 6, 7, 8, 9), base: 5)')
self.assertEqual(repr(a), 'Array[int]((5, 6, 7, 8, 9), base=5)')

a = System.Array.CreateInstance(int, (5,), (15,))
b = System.Array.CreateInstance(int, (5,), (20,))
Expand Down Expand Up @@ -320,9 +340,7 @@ def test_base_negative(self):

# test slice indexing
# 1-dim array [-1, 0, 1]
arr1 = System.Array.CreateInstance(int, (3,), (-1,))
for i in range(-1, 2):
arr1[i] = i
arr1 = System.Array[int]((-1, 0, 1), base=-1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Negative bases! Should have paid more attention in the previous PR. I assume negative indices are not "from the end" in this case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume negative indices are not "from the end" in this case?

Yes. Negative bases caught me by surprise too, I discovered them just a few days ago. I always somehow assumed that bases are zero or positive (to support 1-based arrays). Just to be sure, I wrote a test for negative bases and voilà, they work! This got me thinking about how integer indices are treated in .NET and Python. The overview is in the summary table, but here are some background thoughts.

  • Python indices are always relative. Non-negative indices are relative from the beginning, negative indices are relative from the end. The fact that indices are always relative is a direct consequence of lack of the concept of a base. Without a base, everything is relative.
  • .NET integer indices are always absolute. This is a direct consequence of having a base in the first place. The base determines at which (absolute) index the array actually begins.
  • Since Int32 is an absolute index in .NET, there is a special type to represent a relative index: System.Index. It is only supported on 1D arrays. IronPython does not support it yet (at least not on arrays).
  • In IronPython, we want to support both the Python philosophy and the .NET philosophy seamlessly, so the question is: how should integer indices be treated that is consistent with both .NET and Python? Relative or absolute?
  • Luckilly, for 0-based arrays the answer is straightforward:
    • For non-negative indices, relative == absolute (because absolute == relative + base, and base == 0)
    • For negative indices, well, .NET does not support them, since absolute negative indices are always outside of array bounds for base ==0. So IronPython can adopt the Python convention without creating inconsistencies with .NET.
  • Now, in case of base != 0, relative != absolute. So which convention to adopt? Since Python does not know of bases, and non-zero bases imply absolute indexing, the indexing in IronPython in such case has to be absolute too, and it will not conflict with Python. However:
  • If base > 0, negative indices, if treated as absolute, will always be out-of-bounds. So, for convenience, IronPython treats them as relative, just as in the case of base == 0. In both cases (base == 0 and base > 0), it is an extension to .NET (i.e. Python convention).
  • If base < 0, negative indices cannot be interpreted as relative, because they are used to address some elements in the address space between the base and 0. There is no way around it. So there is a conceptual inconsistency between negative indices with a negative base (absolute, dictated by .NET) and negative indices and 0 base (relative, dictated by Python).
  • The only freedom of choice IronPython has, is how to treat negative indices for base > 0? The .NET convention would always throw, the Python convention would make those arrays behave as 0-based arrays. I chose the latter as more practical, but whatever the choice, it will not remove the inconsistency from the previous point. In practice (if there is any), I expect to see 1-based arrays more than any other base (e.g. for a 3-element array, indices 1 ,2 ,3 are then the same as -3, -2, -1).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. Makes sense.

self.assertEqual(arr1[-1:1], System.Array[int]((-1, 0)))
self.assertEqual(arr1[-2:1], System.Array[int]((-1, 0)))
self.assertEqual(arr1[0:], System.Array[int]((0, 1)))
Expand Down
Loading