Skip to content

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

Merged
ianna merged 10 commits into
scikit-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 into
scikit-hep:mainfrom
ikrommyd:fix-typetracer-indexedoptionarray-to-packed

Conversation

@ikrommyd

@ikrommyd ikrommyd commented Aug 7, 2025

Copy link
Copy Markdown
Member

Fixes #3607

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

@codecov

codecov Bot commented Aug 7, 2025

Copy link
Copy Markdown

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

ikrommyd commented Aug 7, 2025

Copy link
Copy Markdown
Member Author

@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 left a comment

Copy link
Copy Markdown
Collaborator

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?

Comment thread src/awkward/contents/bitmaskedarray.py Outdated
Comment thread src/awkward/contents/unionarray.py
@github-actions

Copy link
Copy Markdown

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 left a comment

Copy link
Copy Markdown
Member

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.

Comment thread src/awkward/contents/bitmaskedarray.py Outdated
Comment thread src/awkward/contents/unionarray.py

@ianna ianna left a comment

Copy link
Copy Markdown
Member

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

Labels

pr-next-release Required for the next release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

4 participants