refactor!: disable iteration of records

[agoose77]

Fixes #1582 by making ak.highlevel.Record non-iterable. Nothing else is changed, as the v2 builder_from_iter supports anything that implements a to_list or tolist method, such as ak.Record and ak.record.Record

[jpivarski]

First, what's the exclamation point for? The "Lint PR" test accepts it as an okay prefix. From a Scheme context, I'd think it's allowing mutation (which all PRs do...).

[jpivarski]

Yes, we should prevent all iteration on Records. (I remember that conversation.)

The way you do this is new to me: I assume that __iter__ = None is different from not defining an __iter__ method (Python apparently doesn't try to call None as a function). I suppose it prevents Python from assuming an iteration algorithm using __getitem__? But then, if Python is going to assume iteration using __getitem__, how does it know what keys/indexes it can pass to __getitem__ are? With Record as it is currently defined and without __iter__, does it try to iterate?

Huh—this is interesting:

>>> class Something:
...     def __getitem__(self, index):
...         return index
... 
>>> for x in Something():
...     print(x)
... 
0
1
2
3
4
5
... (forever)

And __iter__ = None prevents that? Is the error message any better than "iteration on None is not allowed?" Maybe it would be better to define an __iter__ that immediately raises a good error message.

[codecov]

Codecov Report

Merging #1725 (e1d051a) into main (e692946) will increase coverage by 1.01%.
The diff coverage is n/a.

Additional details and impacted files

Impacted Files	Coverage Δ
src/awkward/__init__.py	96.87% <ø> (ø)
src/awkward/_broadcasting.py	93.38% <ø> (ø)
src/awkward/_connect/avro.py	87.17% <ø> (ø)
src/awkward/_connect/cling.py	24.90% <ø> (ø)
src/awkward/_connect/cuda/__init__.py	0.00% <ø> (ø)
src/awkward/_connect/jax/__init__.py	90.47% <ø> (ø)
src/awkward/_connect/jax/_reducers.py	76.92% <ø> (ø)
src/awkward/_connect/numba/arrayview.py	97.77% <ø> (ø)
src/awkward/_connect/numba/builder.py	81.60% <ø> (ø)
src/awkward/_connect/numba/layout.py	84.87% <ø> (ø)
... and 311 more

[agoose77]

The __iter__ = None does indeed disable the legacy iteration that Python otherwise uses. In all my time writing Python, I'd somehow never seen this before, but I suspect I nearly never implemented __getitem__ and not __iter__.

None is a documented special value to disable this behaviour:
https://docs.python.org/3/reference/datamodel.html#special-method-names:~:text=Setting%20a%20special%20method%20to%20None%20indicates%20that%20the%20corresponding%20operation%20is%20not%20available.%20For%20example%2C%20if%20a%20class%20sets%20__iter__()%20to%20None%2C%20the%20class%20is%20not%20iterable%2C%20so%20calling%20iter()%20on%20its%20instances%20will%20raise%20a%20TypeError%20(without%20falling%20back%20to%20__getitem__()).%202

Is the error message any better than "iteration on None is not allowed?"

Yes:

>>> iter(record)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Input In [7], in <cell line: 1>()
----> 1 iter(record)

TypeError: 'Record' object is not iterable

[agoose77]

I'm going to auto-merge this, because you've approved the change, and I'm confident that you'd support following the Python docs in using None here :)

[agoose77]

First, what's the exclamation point for? The "Lint PR" test accepts it as an okay prefix. From a Scheme context, I'd think it's allowing mutation (which all PRs do...).

Sorry @jpivarski, I missed this comment. It implies that the commit / PR contains a breaking change. It's really a case where you're removing a feature, rather than just refactoring internal code that won't really affect the user.