Skip to content

[bug] Ignore padding bytes when tokenizing structured numpy arrays - #273

Merged
timkpaine merged 1 commit into
mainfrom
bug/tokenize-structured-array-padding
Sep 22, 2026
Merged

timkpaine merged 1 commit into
mainfrom
bug/tokenize-structured-array-padding

Conversation

@ptomecek

Copy link
Copy Markdown
Collaborator

Structured numpy dtypes can carry padding between their fields, and those bytes are never written. normalize_token hashed the raw buffer, so two arrays holding identical field values could produce different cache tokens depending on whatever happened to occupy the padding. The easiest way to hit this is np.empty followed by field assignment, which leaves the padding holding unrelated heap contents.

For structured dtypes the handler now recurses per field rather than hashing the buffer, which skips the padding. Arrays with object fields are unaffected — they are already matched by an earlier branch that goes through tolist().

Tokens for structured arrays change as a result, so any cache entry keyed on one is invalidated once.

Structured dtypes can carry padding between their fields, and those bytes
are never written. normalize_token hashed the raw buffer, so two arrays
holding identical field values could produce different cache tokens
depending on whatever happened to occupy the padding. This is easiest to
hit with np.empty followed by field assignment, which leaves the padding
holding unrelated heap contents.

Recurse per field for structured dtypes instead of hashing the buffer.
Arrays with object fields are unaffected, since the earlier hasobject
branch already handles them via tolist() and never sees padding.

Tokens for structured arrays change as a result, so cache entries keyed
on one are invalidated once.

Signed-off-by: Pascal Tomecek <pascal.tomecek@cubistsystematic.com>
@ptomecek
ptomecek marked this pull request as ready for review September 21, 2026 23:25
@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

    1 files  ±0      1 suites  ±0   2m 18s ⏱️ -43s
1 363 tests +3  1 361 ✅ +3  2 💤 ±0  0 ❌ ±0 
1 369 runs  +3  1 367 ✅ +3  2 💤 ±0  0 ❌ ±0 

Results for commit 03a8387. ± Comparison against base commit e65ff57.

♻️ This comment has been updated with latest results.

@codecov

codecov Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.59%. Comparing base (e65ff57) to head (03a8387).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #273   +/-   ##
=======================================
  Coverage   93.58%   93.59%           
=======================================
  Files         176      176           
  Lines       20586    20613   +27     
  Branches     1359     1361    +2     
=======================================
+ Hits        19265    19292   +27     
  Misses       1048     1048           
  Partials      273      273           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

@timkpaine
timkpaine merged commit bb4ab7e into main Sep 22, 2026
20 checks passed
@timkpaine
timkpaine deleted the bug/tokenize-structured-array-padding branch September 22, 2026 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants