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: update codebase to Awkward 2.0.0. #156

Merged
merged 4 commits into from
Dec 13, 2022

Conversation

jpivarski
Copy link
Member

I wasn't able to test locally. I think I got all the right changes. Let's see what CI thinks.

@jpivarski jpivarski linked an issue Dec 12, 2022 that may be closed by this pull request
@jpivarski jpivarski self-assigned this Dec 12, 2022
@jpivarski jpivarski requested a review from jmduarte December 12, 2022 20:23
@jpivarski
Copy link
Member Author

Oh wait—I have to remove debugging code.

@jpivarski
Copy link
Member Author

I guess I read the pytest output from Round 1 incorrectly: there was one error causing three test failures. With that error fixed, the other two didn't need to be debugged.

So it's done!

To explain the changes, most of them are just renaming, but in some of them, I replaced a lot of

isinstance(layout, (ak.layouts.ThisArray, ak.layouts.ThatArray))

with

layout.is_this or layout.is_that

These "is_*" booleans are categories of similar array types. All of the integer-sized type variations are gone (e.g. ListArray64 versus ListArray32), which would have simplified the isinstance calls on its own, but these "is_*" also identify commonly selected groups (e.g. is_list is True for ListArray, ListOffsetArray, and RegularArray).

I removed code that depends on now-nonexistent types, PartitionedArray and VirtualArray. For the tests, however, I just changed the PartitionedArray test to work on a concatenated array and the VirtualArray test to work on eager data.

I also introduced use of the copy method to change one field of a layout node, rather than rebuilding the whole layout node. (If I hadn't done that, I would also have needed to remove references to the identities, which have been removed.)

I scanned over the changes once again, and yeah: it was mostly renaming.

Copy link
Collaborator

@jmduarte jmduarte left a comment

Choose a reason for hiding this comment

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

LGTM!

@jmduarte jmduarte merged commit 9c05c0b into main Dec 13, 2022
@jmduarte jmduarte deleted the jpivarski/update-to-awkward-2-0-0 branch December 13, 2022 03:35
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.

Upgrade to awkward 2.0.0
2 participants