added a mapper that maps the indexes of a view to the indices of a container by alexandrehoffmann · Pull Request #2880 · xtensor-stack/xtensor

alexandrehoffmann · 2025-12-12T15:09:53Z

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.

Description

The index_mapper class provides functionality to convert indices from a view's coordinate system
to the corresponding indices in the underlying container. This is particularly useful for views
that contain integral slices (fixed indices), as these slices reduce the dimensionality of the view.

Example:

xt::xarray<double> a = xt::arange(24).reshape({2, 3, 4});
auto view1 = xt::view(a, 1, xt::all(), xt::all());  // Fixed first dimension
index_mapper<decltype(view1)> mapper;

// Map view indices (i,j) to container indices (1,i,j)
double val = mapper.map(a, view1, 0, 0);  // Returns a(1, 0, 0)
double val2 = mapper.map(a, view1, 1, 2); // Returns a(1, 1, 2)

Copilot

Pull request overview

This PR introduces a new index_mapper class that enables conversion of indices from a view's coordinate system to the underlying container's coordinate system. This is particularly useful when working with views that contain fixed dimensions (integral slices), which reduce the dimensionality compared to the original container.

Key Changes:

Implements index_mapper template class with support for xview types
Provides map() and map_at() methods for index conversion with and without bounds checking
Adds comprehensive test coverage for 2D and 3D views with various slicing patterns

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 22 comments.

File	Description
`include/xtensor/views/index_mapper.hpp`	New header file implementing the `index_mapper` class with template metaprogramming to handle view-to-container index mapping
`test/test_xview.cpp`	Adds test cases for `index_mapper` functionality covering simple 2D views and 3D views with various slice configurations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

include/xtensor/views/index_mapper.hpp

Copilot · 2025-12-12T15:16:53Z

include/xtensor/views/index_mapper.hpp

+     * @throws Assertion failure if `i != 0` for integral slices.
+     * @throws Assertion failure if `i >= slice.size()` for non-integral slices.


The assertion message is misleading in the documentation. The assertion checks that the index is within bounds (i < slice.size()), but the documentation states it "throws Assertion failure if i >= slice.size()". While technically correct, assertions don't throw exceptions in the traditional sense - they abort the program in debug builds. Consider updating the documentation to clarify this behavior or use proper exception handling if runtime bounds checking is desired.

Suggested change

* @throws Assertion failure if `i != 0` for integral slices.

* @throws Assertion failure if `i >= slice.size()` for non-integral slices.

* @note This function uses assertions to enforce preconditions:

* - Aborts with assertion failure if `i != 0` for integral slices.

* - Aborts with assertion failure if `i >= slice.size()` for non-integral slices.

* These are not exceptions and will terminate the program in debug builds if triggered.

Copilot · 2025-12-12T15:16:53Z

include/xtensor/views/index_mapper.hpp

+    value_type map_at(const UnderlyingContainer& container, const view_type& view, const Indices... indices) const 
+        requires(sizeof...(Indices) == n_free);
+
+    constexpr size_t dimension() const { return n_free; }


The dimension() method returns the number of free (non-integral) dimensions, but the method name is ambiguous. It's unclear whether this returns the view's dimension or the container's dimension. Consider renaming to free_dimensions() or view_dimension() to make the purpose clearer.

Suggested change

constexpr size_t dimension() const { return n_free; }

constexpr size_t free_dimensions() const { return n_free; }

test/test_xview.cpp

include/xtensor/views/index_mapper.hpp

…e number of specified slices. If more indices are provided we assume an slice. The underlying container handles the excess/missing indices

JohanMabille

It would be better to have a single public and a single private sections in the index_mapper class (the public being the first one so we can quickly browse its API when we need).

JohanMabille · 2026-01-06T16:16:24Z

include/xtensor/views/index_mapper.hpp

+        static constexpr size_t nb_integral_slices = (std::is_integral_v<Slices> + ...);
+
+        /// @brief Number of slices that are xt::newaxis (insert a  dimension)
+        static constexpr size_t nb_new_axis_slices = (xt::detail::is_newaxis<Slices>::value + ...);


I think we can take the opportunity to define xt::detail::is_new_axis_v constant (just list std::is_xxx_v to avoid the need for ::value).

JohanMabille · 2026-01-06T16:16:44Z

include/xtensor/views/index_mapper.hpp

+
+        /// @brief Helper type alias for the I-th slice type
+        template <size_t I>
+        using ith_slice_type = std::tuple_element_t<I, std::tuple<Slices...>>;


Naming: I think the ith prefix can be removed.

JohanMabille · 2026-01-06T16:16:52Z

include/xtensor/views/index_mapper.hpp

+
+        /// @brief True if the I-th slice is an integral slice (fixed index)
+        template <size_t I>
+        static consteval bool is_ith_slice_integral();


JohanMabille · 2026-01-06T16:16:58Z

include/xtensor/views/index_mapper.hpp

+
+        /// @brief True if the I-th slice is a newaxis slice
+        template <size_t I>
+        static consteval bool is_ith_slice_new_axis();


JohanMabille · 2026-01-06T16:17:41Z

include/xtensor/views/index_mapper.hpp

+            using new_axis_type = typename indices_sequence_helper<first + 1, bound, indices...>::Type;
+
+            // NOTE: is_ith_slice_new_axis works even if first >= sizeof...(Slices)
+            using Type = std::conditional_t<is_ith_slice_new_axis<first>(), new_axis_type, not_new_axis_type>;


naming convention: should be type.

JohanMabille · 2026-01-06T16:24:41Z

include/xtensor/views/index_mapper.hpp

+         * @return value_type The value at the mapped location in the container.
+         */
+        template <size_t n_indices, size_t... Is>
+        value_type map_all_indices(


The return type should be const_reference. Also, a non-const overload should be provided.

So the compiler can't chose between the const and non const overload.

// Version 1: Takes const container, returns const_reference template <size_t n_indices, size_t... Is> const_reference map_at_all_indices( const UnderlyingContainer& container, // const reference const view_type& view, std::index_sequence<Is...>, const std::array<size_t, n_indices>& indices ) const; // Version 2: Takes non-const container, returns reference template <size_t n_indices, size_t... Is> reference map_at_all_indices( UnderlyingContainer& container, // non-const reference const view_type& view, std::index_sequence<Is...>, const std::array<size_t, n_indices>& indices ) const;

Should I use a prefix or postfix to differentiate the two implementations ?

Although I don't really like this option, let's do it for finishing the implementation of the mapper, and we can later understand and fix this issue.

Fixed cmap for const and map for non-const

JohanMabille · 2026-01-06T16:24:55Z

include/xtensor/views/index_mapper.hpp

+         * @throws std::out_of_range if any index is out of bounds.
+         */
+        template <size_t n_indices, size_t... Is>
+        value_type map_at_all_indices(


JohanMabille · 2026-01-06T16:26:15Z

include/xtensor/views/index_mapper.hpp

+         */
+        template <std::integral... Indices>
+        value_type
+        map(const UnderlyingContainer& container, const view_type& view, const Indices... indices) const;


JohanMabille · 2026-01-06T16:26:23Z

include/xtensor/views/index_mapper.hpp

+         */
+        template <std::integral... Indices>
+        value_type
+        map_at(const UnderlyingContainer& container, const view_type& view, const Indices... indices) const;


JohanMabille · 2026-01-06T16:28:52Z

include/xtensor/views/index_mapper.hpp

+{
+
+    template <class UndefinedView>
+    struct index_mapper;


This should be declared as class to avoid a warning.

…ator() and .at() calls with a unified subfunction. A

JohanMabille · 2026-01-29T13:05:54Z

Thanks!

Alexandre Hoffmann added 2 commits December 12, 2025 14:38

feurst commiteuh

66dd786

added comments

f818520

alexandrehoffmann requested review from JohanMabille and Copilot December 12, 2025 15:10

Copilot started reviewing on behalf of alexandrehoffmann December 12, 2025 15:10 View session

Copilot AI reviewed Dec 12, 2025

View reviewed changes

Alexandre Hoffmann added 3 commits December 12, 2025 16:22

added pre-commit

89accf2

added pre-commit 2

1d46493

mapper works even if when the user provides more/less indices than th…

4c62edc

…e number of specified slices. If more indices are provided we assume an slice. The underlying container handles the excess/missing indices

JohanMabille requested changes Jan 6, 2026

View reviewed changes

refacto, the mapper handles both const and non-const, as well as oper…

e3f1e74

…ator() and .at() calls with a unified subfunction. A

JohanMabille approved these changes Jan 29, 2026

View reviewed changes

JohanMabille merged commit 18f6524 into xtensor-stack:master Jan 29, 2026
19 of 20 checks passed

		* @throws Assertion failure if `i != 0` for integral slices.
		* @throws Assertion failure if `i >= slice.size()` for non-integral slices.

-     * @throws Assertion failure if `i != 0` for integral slices.
-     * @throws Assertion failure if `i >= slice.size()` for non-integral slices.
+     * @note This function uses assertions to enforce preconditions:
+     *       - Aborts with assertion failure if `i != 0` for integral slices.
+     *       - Aborts with assertion failure if `i >= slice.size()` for non-integral slices.
+     *       These are not exceptions and will terminate the program in debug builds if triggered.

	constexpr size_t dimension() const { return n_free; }
	constexpr size_t free_dimensions() const { return n_free; }

Conversation

alexandrehoffmann commented Dec 12, 2025

Checklist

Description

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JohanMabille left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohanMabille Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohanMabille commented Jan 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

JohanMabille Jan 22, 2026 •

edited

Loading