fix: keep track of buffer key instead of field path for VirtualNDArrays & PlaceholderArrays

[pfackeldey]

There's more information in the buffer key than in the field path. Also, 3rd party libraries (like coffea) implement their own buffer keys, which makes this a more useful variable to keep track of than our manually build field path.

[github-actions]

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

[codecov]

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.73%. Comparing base (b749e49) to head (1efc3fb).
⚠️ Report is 488 commits behind head on main.

Files with missing lines	Patch %	Lines
src/awkward/_nplikes/placeholder.py	71.42%	2 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
src/awkward/_nplikes/array_module.py	95.25% <100.00%> (+8.87%)	⬆️
src/awkward/_nplikes/virtual.py	90.86% <100.00%> (ø)
src/awkward/operations/ak_from_buffers.py	94.17% <100.00%> (+0.05%)	⬆️
src/awkward/_nplikes/placeholder.py	66.40% <71.42%> (-0.60%)	⬇️

... and 196 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ianna]

@pfackeldey - Looks great! Thanks for implementing it. It does make more sense to use the buffer keys. Are you planning to add the tests? Thanks.

[pfackeldey]

Do you have a concrete suggestion what to test here?
I don't think it's useful to check if .buffer_key is set, similarly how we also don't test if PlaceholderArray(...) has .shape or .dtype set.

Also, I was wondering if it makes sense to forward the buffer_key to methods that return a lazified operation, e.g. .T or .byteswap or nplike.asarray?

[ikrommyd]

I think it definitely does make sense to forward the key for things that keep the array "intact" like asarray for example. I'm not 100% sure about things that change the shape like .T. It depends on what we wanna keep track. Do we want to keep track the original key that the buffer was created from so that we can use caching? If yes, then we can forward it for .T too for example. If the buffer key is meant to signify that "I am the original buffer array" then no.

[pfackeldey]

I think it definitely does make sense to forward the key for things that keep the array "intact" like asarray for example. I'm not 100% sure about things that change the shape like .T. It depends on what we wanna keep track. Do we want to keep track the original key that the buffer was created from so that we can use caching? If yes, then we can forward it for .T too for example. If the buffer key is meant to signify that "I am the original buffer array" then no.

yeah, I was hoping for input as I am unsure here. I just forwarded it everywhere now, and I think that's reasonable... also ".T" is useless for 1D arrays, getitem's are usually a view into the original buffer anyways (except for fancy indexing/slicing), byteswap is the same data but different interpretation... so all-in-all that led me to forwarding it everywhere.

[ikrommyd]

I honestly don't know if .T is used anywhere and indeed it doesn't do anything for 1D buffers (and is also a view) and yeah with everything else where we "lazify" the method by creating new virtual arrays, it seems to me that the key should be forwarded like you are doing now.

[ikrommyd]

Question, does the fact that you are changing it for placeholders have any implications on uproot.dask/dask-awkward?

[pfackeldey]

Question, does the fact that you are changing it for placeholders have any implications on uproot.dask/dask-awkward?

It should not. the field path was only used for better error messages inside the PlaceholderArray. We can give the same useful errors now, but with the buffer key instead.

[ikrommyd]

I think this is generally good!
Do you want to add a test where you make an array with virtual buffers and then test that the buffers inside the layout have the expected buffer keys? It can be a very simple structure with a few buffers.

I also think that we probably should forward the buffer keys in every operation that we create a new virtual array like you are doing now. I think the point is to know "which original buffer to do you need to materialize this array?" This is the correct logic if we want to use these keys to cache stuff for example.

[ikrommyd]

LGTM!