Skip to content

fix: form and type equality bugs#4101

Open
henryiii wants to merge 4 commits into
mainfrom
henryiii/fix-form-type-equality
Open

fix: form and type equality bugs#4101
henryiii wants to merge 4 commits into
mainfrom
henryiii/fix-form-type-equality

Conversation

@henryiii

@henryiii henryiii commented Jun 10, 2026

Copy link
Copy Markdown
Member

🤖 AI text below 🤖

Summary

This PR fixes eight bugs and one performance issue in form/type equality and union parameter lookup:

  • NumpyForm._is_equal_to (src/awkward/forms/numpyform.py): missing _inner_shape comparison caused NumpyForm("float64", (2,)) == NumpyForm("float64", (3,)) to be True. Fixed with unknown_length-safe per-element comparison.
  • RegularForm._is_equal_to (src/awkward/forms/regularform.py) and RegularType._is_equal_to (src/awkward/types/regulartype.py): self._size == other._size raised TypeError when either side was unknown_length (its __eq__ raises on known values). Fixed with the guarded pattern from types/arraytype.py.
  • ListType._is_equal_to (src/awkward/types/listtype.py): content comparison used == (which calls is_equal_to with default all_parameters=False), dropping the caller's flag. Fixed by calling _is_equal_to(other._content, all_parameters) directly, matching OptionType.
  • UnionMeta.purelist_parameters (src/awkward/_meta/unionmeta.py): unconditional return out inside the for key in keys loop caused only the first key to ever be tried, making __list__-based behavior lookup on unions fail when __record__ was absent. Fixed so the loop continues to the next key when the current one yields no consistent value.
  • RecordForm._is_equal_to (src/awkward/forms/recordform.py) and RecordType._is_equal_to (src/awkward/types/recordtype.py): per-field other.content(field) call used list.index → O(n²) for n fields. Fixed by building a field→content dict for other once before the loop.
  • ListForm.__init__ (src/awkward/forms/listform.py): error message for invalid stops was a copy-paste of the starts message. Fixed to name and show stops.
  • IndexedForm.__init__ (src/awkward/forms/indexedform.py): content=None default always fails validation immediately after; removed the default to make the required-ness explicit.
  • ListOffsetForm.__init__ (src/awkward/forms/listoffsetform.py): the only list-like form that did not validate content is a Form; added the check matching sibling forms.

AI assistance

This PR was produced with Claude Code (automated multi-agent review), an AI-assisted workflow. Per CONTRIBUTING.md, this is disclosed here.

Test plan

New regression tests in tests/test_4101_fix_form_type_equality.py cover one focused case per distinct bug:

  • NumpyForm inner_shape inequality (different sizes → not equal)
  • NumpyForm inner_shape with unknown_length (compatible with any size)
  • NumpyForm inner_shape rank mismatch (different number of dimensions → not equal)
  • RegularForm/RegularType unknown_length vs. known comparison (no TypeError)
  • ListType all_parameters flag propagation to content
  • UnionMeta.purelist_parameters second-key resolution (core bug)
  • UnionMeta.purelist_parameters inconsistency → None (preserved behaviour)

Existing tests in tests/test_2368_type_is_equal.py, tests/test_2426_is_equal_to.py, tests/test_0914_types_and_forms.py, tests/test_0348_form_keys.py, tests/test_2425_forms_from_type.py all remain green.

prek -a --quiet passes.

🤖 Generated with Claude Code

henryiii added a commit that referenced this pull request Jun 10, 2026
Covers NumpyForm inner_shape inequality, RegularForm/RegularType unknown_length
comparison, ListType all_parameters propagation, and union purelist_parameters
second-key resolution.

Assisted-by: ClaudeCode:claude-sonnet-4-6
@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.34%. Comparing base (712dac0) to head (09f176d).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/awkward/forms/listoffsetform.py 50.00% 1 Missing ⚠️
src/awkward/forms/recordform.py 90.90% 1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (90.47%) is below the target coverage (98.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
Files with missing lines Coverage Δ
src/awkward/_meta/unionmeta.py 95.50% <100.00%> (+1.25%) ⬆️
src/awkward/forms/indexedform.py 75.29% <ø> (ø)
src/awkward/forms/listform.py 74.66% <ø> (ø)
src/awkward/forms/numpyform.py 78.74% <100.00%> (ø)
src/awkward/forms/regularform.py 84.28% <100.00%> (+0.22%) ⬆️
src/awkward/types/listtype.py 95.83% <ø> (ø)
src/awkward/types/recordtype.py 76.51% <100.00%> (+0.15%) ⬆️
src/awkward/types/regulartype.py 96.49% <100.00%> (+0.06%) ⬆️
src/awkward/forms/listoffsetform.py 91.17% <50.00%> (-1.25%) ⬇️
src/awkward/forms/recordform.py 94.17% <90.90%> (-0.51%) ⬇️

... and 2 files with indirect coverage changes

@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.

@henryiii — looks great! Perhaps worth adding the following to the Agents.md:

nox > python -m pip install 'pylint~=3.3.0'
nox > pylint src

henryiii and others added 4 commits June 11, 2026 15:41
- NumpyForm._is_equal_to: compare _inner_shape in addition to _primitive,
  using unknown_length-safe per-element comparison
- RegularForm._is_equal_to and RegularType._is_equal_to: guard size comparison
  against unknown_length to avoid TypeError (unknown_length.__eq__ raises on
  known values)
- ListType._is_equal_to: propagate all_parameters flag to content comparison
  instead of calling __eq__ (which defaults to all_parameters=False)
- UnionMeta.purelist_parameters: fix unconditional return after the first key
  so later keys are tried when the first yields no consistent answer; this
  fixes __list__-based behavior lookup on union arrays
- RecordForm._is_equal_to and RecordType._is_equal_to: build a field→content
  dict for the other record once before the loop, reducing O(n²) to O(n)
- ListForm.__init__: fix stops error message (was copy-pasting starts message)
- IndexedForm.__init__: remove misleading content=None default that always
  fails validation
- ListOffsetForm.__init__: add content validation matching sibling forms

Assisted-by: ClaudeCode:claude-sonnet-4-6
Covers NumpyForm inner_shape inequality, RegularForm/RegularType unknown_length
comparison, ListType all_parameters propagation, and union purelist_parameters
second-key resolution.

Assisted-by: ClaudeCode:claude-sonnet-4-6
Removed tests that do not exercise a bug (positive equality, known-size
inequality, first-key-wins ordering). Renamed file from hyphens to
underscores to satisfy the test-name validator hook. Removed section-header
comments from the test file.

Assisted-by: ClaudeCode:claude-sonnet-4-6
@henryiii henryiii force-pushed the henryiii/fix-form-type-equality branch from b19b951 to 09f176d Compare June 11, 2026 19:43
@TaiSakuma TaiSakuma added the type/fix PR title type: fix (set automatically) label Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/fix PR title type: fix (set automatically)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants