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

fix: placeholder arrays in record __repr__ #3342

Merged
merged 2 commits into from
Dec 17, 2024

Conversation

pfackeldey
Copy link
Collaborator

@pfackeldey pfackeldey commented Dec 14, 2024

Follow-up of #3341
This fixes the __repr__ for Record arrays that contain PlaceholderArrays:

import awkward as ak
import numpy as np
from awkward._nplikes.numpy import Numpy
from awkward._nplikes.placeholder import PlaceholderArray

numpy = Numpy.instance()

form = {
    "class": "RecordArray",
    "fields": [
        "foo"
    ],
    "contents": [
        {
            "class": "NumpyArray",
            "primitive": "int64",
            "form_key": "node1"
        }
    ],
    "form_key": "node0"
}

# without this PR
ak.from_buffers(form, 3, {"node1-data": PlaceholderArray(numpy, (3,), np.int64)}, highlevel=True)
# ... TypeError: PlaceholderArray supports only trivial slices, not int

# with this PR
ak.from_buffers(form, 3, {"node1-data": PlaceholderArray(numpy, (3,), np.int64)}, highlevel=True)
# <Array [{foo: ??}, {foo: ??}, {foo: ??}] type='3 * {foo: int64}'>

Another cool thing: It can show mixtures of e.g. PlaceholderArrays and numpy (or others) now too:

form = {
    "class": "RecordArray",
    "fields": [
        "foo",
        "bar"
    ],
    "contents": [
        {
            "class": "NumpyArray",
            "primitive": "int64",
            "form_key": "node1"
        },
        {
            "class": "NumpyArray",
            "primitive": "int64",
            "form_key": "node2"
        }
    ],
    "form_key": "node0"
}

containers = {
    "node1-data": PlaceholderArray(numpy, (3,), np.int64),
    "node2-data": np.array([4,5,6], np.int64),
}
ak.from_buffers(form, 3, containers, highlevel=True)
# <Array [{foo: ??, bar: 4}, {...}, {...}] type='3 * {foo: int64, bar: int64}'>

Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

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

Wait, so this was already in

def get_at(data: Content, index: int):
if data._layout._is_getitem_at_placeholder():
return PlaceholderValue()

and I just overlooked the get_field case in #3019? That's just my mistake, then. The get_at and get_field are supposed to be parallel to each other; one for rows and the other for columns.

It is definitely the right fix.

@pfackeldey
Copy link
Collaborator Author

Yes, that's my understanding aswell. 👍

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 this pull request may close these issues.

2 participants