fix: ak.array_equal on unions with reordered index into list child - #4321
aashirvad08 wants to merge 7 commits into
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (96.42%) is below the target coverage (98.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files
|
|
@aashirvad08 Thank you for the PR. |
|
The documentation preview is ready to be viewed at https://awkward-array.org/doc/pr/4321/ |
| @@ -0,0 +1,154 @@ | |||
| # BSD 3-Clause License; see https://github.com/scikit-hep/awkward/blob/main/LICENSE | |||
|
|
|||
| from __future__ import annotations | |||
There was a problem hiding this comment.
| from __future__ import annotations |
This is not used and can be removed.
|
This PR doesn't fully resolve #4316 but fixes the reported reproducers. I can approve the PR if you replace "Closes #4316" in the PR description with something that doesn't automatically close the issue, for example, "Part of #4316". In the next comment, I'll post which part of the issue this PR doesn't fix. If you would prefer this PR to close the issue, you can push more commits to fix the rest. |
|
🤖 The text below was written by Claude. What remains of #4316. The false negative remains when the record under the list is nullable, which is the pyarrow default for a struct. There, the lazy carry turns an import pyarrow as pa
import awkward as ak
def dense(offsets, lists):
return ak.from_arrow(
pa.UnionArray.from_dense(
pa.array([1, 1, 0], pa.int8()),
pa.array(offsets, pa.int32()),
[
pa.array([2], pa.int64()),
pa.array(lists, pa.list_(pa.struct([("x", pa.int64())]))),
],
)
)
a = dense([0, 1, 0], [[{"x": 1}], []])
b = dense([1, 0, 0], [[], [{"x": 1}]])
assert a.tolist() == b.tolist()
ak.array_equal(a, b) # FalseThe comment in One result changes outside #4316. With a categorical import numpy as np
import awkward as ak
def union(child):
return ak.Array(
ak.contents.UnionArray(
ak.index.Index8(np.array([0, 1, 1], np.int8)),
ak.index.Index64(np.array([0, 0, 1])),
[ak.contents.NumpyArray(np.array([1.5])), child],
)
)
def records(values):
return ak.contents.RecordArray([ak.contents.NumpyArray(np.array(values))], ["y"])
categorical = ak.contents.IndexedArray(
ak.index.Index64(np.array([1, 0])),
records([5, 6]),
parameters={"__array__": "categorical"},
)
ak.array_equal(union(categorical), union(records([6, 5]))) # main: True, this PR: FalseThe same comparison outside a union returns False on both main and this PR, so the new result agrees with it. 🤖 Generated with Claude Code |
|
Thanks, both comments addressed. I’ve fixed the remaining case and added regression tests covering both issues. |
Fixes
ak.array_equalreturningFalsefor equal unions when a reordered index points into a list childThe issue was caused by derived nodes having different concrete types even though their packed representations were equivalent
This normalizes those nodes before the
same_content_typescheck and adds regression tests covering reordered indices and nested list-record cases.Closes #4316