Skip to content

Fix drop slice index ordering and repeated normalization - #2935

Open
allenflux wants to merge 1 commit into
xtensor-stack:masterfrom
allenflux:oct11-fix-drop-index-order
Open

allenflux wants to merge 1 commit into
xtensor-stack:masterfrom
allenflux:oct11-fix-drop-index-order

Conversation

@allenflux

Copy link
Copy Markdown

Checklist

  • The title and commit message(s) are descriptive.
  • Small commits made to fix your PR have been squashed to avoid history pollution.
  • Tests have been added for new features or bug fixes.
  • API of new functions and classes are documented. (No new API.)

Description

Fixes #2595.

For an input {0, 1, 2, 3, 4, 5}, xt::view(input, xt::drop(2, 0)) currently evaluates to {2, 3, 3, 4} rather than {1, 3, 4, 5}. The cumulative offset calculation assumes that excluded indices are sorted. Sort the normalized indices before building offsets, so mixed positive and negative indices also use their final positions. Clear the offset map before rebuilding it to avoid retaining keys when a drop slice is normalized again for another shape.

The regression tests cover variadic and container indices, mixed positive/negative indices on a two-dimensional view, iteration, assignment, and repeated normalization. An unordered keep slice retains its requested order.

Validation on macOS arm64 with Apple Clang 21 and C++20:

  • The three new regression cases fail against the original header (40 failed assertions) and pass with the fix (50 passed assertions).
  • Official complete test_xtensor_lib target: 1,253 test cases and 19,767 assertions passed, including optional JSON/MIME tests.
  • test_xview and test_xdynamic_view passed in Release, with combined AddressSanitizer/UndefinedBehaviorSanitizer, and with default column-major layout.
  • Repository-pinned clang-format 17.0.6 checked the changed lines; git diff --check passed.

AI assistance was used for investigation, the implementation and regression tests, local validation, and this draft description. Duplicate and out-of-range drop indices are outside this change's scope.

@allenflux
allenflux marked this pull request as ready for review October 11, 2026 06:57

This branch has not been deployed

No deployments
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.

xt::drop doesn't work when indices are out of order

1 participant