fix: virtual array deep copy generator reference + better error message for assert_never#3644
Merged
ianna merged 4 commits intoscikit-hep:mainfrom Sep 10, 2025
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
🚀 New features to boost your workflow:
|
Collaborator
|
After some discussion, this is high priority for coffea. If this can be approved and merged expediently, with a corresponding patch release, I would appreciate that with my coffea hat on. That, or I can take responsibility for it since I have merge rights on this repository. |
|
The documentation preview is ready to be viewed at http://preview.awkward-array.org.s3-website.us-east-1.amazonaws.com/PR3644 |
ianna
requested changes
Sep 9, 2025
assert_neverassert_never
Collaborator
Author
|
Anything we can do about the GPU ci that's apparently frozen? |
Collaborator
|
@ikrommyd it just takes a while to get a slot! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While deep copying virtual arrays, we used to do
lambda: copy.deepcopy(self._generator(), memo)as the new generator.The problem is that this apparently keeps a reference to
selfand not toself._generator. So when you make a copy and materialize the parent array first, its generator is set toassert_neverinmaterialize, which effectively changes the generator ofself. Therefore you cannot materialize the copy anymore becauseself._generatoris nowassert_never.By extracting
self._generatorinto a variable first, this keeps the right reference to the original generator and now you can materialize both the parent and the copied array in any order you want.While we're at it, we add a better error message for
assert_never, similar to the one for placeholder materialization.