Skip to content

fix: make to_packed work for typetracer backed IndexedOptionArray, BitMaskedArray, and UnionArray#3608

Merged
ianna merged 10 commits intoscikit-hep:mainfrom
ikrommyd:fix-typetracer-indexedoptionarray-to-packed
Sep 6, 2025
Merged

fix: make to_packed work for typetracer backed IndexedOptionArray, BitMaskedArray, and UnionArray#3608
ianna merged 10 commits intoscikit-hep:mainfrom
ikrommyd:fix-typetracer-indexedoptionarray-to-packed

Conversation

@ikrommyd
Copy link
Copy Markdown
Collaborator

@ikrommyd ikrommyd commented Aug 7, 2025 •
edited
Loading

Fixes #3607

This PR also adds testing for to_packed for all layouts with typetracer backend which wasn't tested so far.

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 7, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.67%. Comparing base (b749e49) to head (3cd7f3b).
⚠️ Report is 417 commits behind head on main.

Additional details and impacted files
Files with missing lines Coverage Δ
src/awkward/contents/bitmaskedarray.py 70.02% <100.00%> (-0.41%) ⬇️
src/awkward/contents/bytemaskedarray.py 87.64% <100.00%> (-2.02%) ⬇️
src/awkward/contents/indexedoptionarray.py 89.09% <100.00%> (+0.87%) ⬆️
src/awkward/contents/unionarray.py 86.33% <100.00%> (+1.08%) ⬆️

... and 193 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ikrommyd ikrommyd changed the title fix: make to_packed work for typetracer backed IndexedOptionArray fix: make to_packed work for typetracer backed IndexedOptionArray, BitMaskedArray, and UnionArray Aug 7, 2025
@ikrommyd
Copy link
Copy Markdown
Collaborator Author

ikrommyd commented Aug 7, 2025 •
edited
Loading

@agoose77 could you please take a quick look at my UnionArray changes because to_packed there had zero typetracer handling. I don't know if the If X is not unknown_length statements I added there are correct or if I'm missing something and it requires some extra checks in the typetracer case. What's certain is we definitely can't do data dependent decisions with typetracers but I don't know if it needs extra handling there.

pfackeldey
pfackeldey approved these changes Aug 7, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@pfackeldey pfackeldey left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks! This does look good to me 👍

This PR is ready from my side :)

Let's see if @agoose77 has some comments before we merge, ok?

agoose77
agoose77 reviewed Aug 9, 2025
View reviewed changes
agoose77
agoose77 reviewed Aug 9, 2025
View reviewed changes
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 26, 2025

The documentation preview is ready to be viewed at http://preview.awkward-array.org.s3-website.us-east-1.amazonaws.com/PR3608

@ianna ianna added the pr-next-release Required for the next release label Sep 4, 2025
ianna
ianna requested changes Sep 5, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ianna ianna left a comment

Choose a reason for hiding this comment

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

@ikrommyd - I still see some unresolved comments. Do you think we could on the comments agree soon-ish? Thanks.

ianna
ianna approved these changes Sep 6, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ianna ianna left a comment

Choose a reason for hiding this comment

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

@ikrommyd - thanks for addressing the comments! I'm merging it for the upcoming release.

@ianna ianna merged commit df7fbff into scikit-hep:main Sep 6, 2025
46 checks passed
@ikrommyd ikrommyd deleted the fix-typetracer-indexedoptionarray-to-packed branch September 6, 2025 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@agoose77 agoose77 agoose77 left review comments

@ianna ianna ianna approved these changes

@pfackeldey pfackeldey pfackeldey approved these changes

Assignees

No one assigned

Labels

pr-next-release Required for the next release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

to_packed doesn't work on typetracer backed IndexOptionArray, BitMaskedArray, and UnionArray

4 participants

@ikrommyd @agoose77 @ianna @pfackeldey