Repository: apytypes/apytypes (language: python)

Issue:
C++: hoist common member functions out of arrays
This is an incomplete list of candidate member functions in `APyFixedArray` and `APyFloatArray` that are near identical source-code-wise, and should probably be hoisted to a common array base class.

* [x] `transpose`
* [x] `rehsape`
* [x] `flatten`
* [x] `ravel`
* [x] `swapaxes` 
* [x] `squeeze`
* [x] `__getitem__` (+ family of helpers)
* [x] `size`
* [x] `ndim`
* [x] `python_get_shape`
* [x] `broadcast_to`
* [x] `broadcast_to_python`
* [ ] ~~`identity`~~
* [ ] ~~`eye`~~
* [x] `diagonal`
* [x] `full`
* [ ] ~~`zeros`~~
* [ ] ~~`ones`~~



Discussion / hints:

Fix patch:
diff --git a/src/apyarray.h b/src/apyarray.h
index d8e52a637..19793eceb 100644
--- a/src/apyarray.h
+++ b/src/apyarray.h
@@ -30,6 +30,7 @@ template <typename T, typename ARRAY_TYPE> class APyArray : public APyBuffer<T>
     using APyBuffer<T>::_ndim;
     using APyBuffer<T>::_data;
     using APyBuffer<T>::_itemsize;
+    using APyBuffer<T>::_nitems;
 
     /* ****************************************************************************** *
      * *                    `__getitem__` family of methods                         * *
@@ -362,6 +363,83 @@ template <typename T, typename ARRAY_TYPE> class APyArray : public APyBuffer<T>
         return broadcast_to(cpp_shape_from_python_shape_like(shape));
     }
 
+    /* ****************************************************************************** *
+     * *                     `reshape` family of methods                            * *
+     * ****************************************************************************** */
+
+    //! Attempt to reshape `*this` into `new_shape`, resolving any -1 dimension. Returns
+    //! the resolved new shape on success.
+    std::vector<std::size_t> try_reshape(nb::tuple new_shape) const
+    {
+        std::vector<std::size_t> new_shape_vec;
+        std::size_t index = 0;
+        std::size_t reshape_size = 1;
+        std::size_t neg_one_count = 0;
+        std::ptrdiff_t neg_one_pos = -1;
+        for (auto it = new_shape.begin(); it != new_shape.end(); ++it, ++index) {
+            std::ptrdiff_t current_value = nb::cast<std::ptrdiff_t>(*it);
+            if (current_value == -1) {
+                neg_one_count++;
+                neg_one_pos = index;        // store the position of -1
+                new_shape_vec.push_back(1); // temporarily push 1, replaced later
+            } else if (current_value < -1) {
+                std::string error_msg = fmt::format(
+                    "{}.reshape: array dimensions must be greater than or equal to -1",
+                    ARRAY_TYPE::ARRAY_NAME
+                );
+                throw nb::value_error(error_msg.c_str());
+            } else {
+                std::size_t c_size_t = nb::cast<std::size_t>(*it);
+                new_shape_vec.push_back(c_size_t);
+                reshape_size *= c_size_t;
+            }
+        }
+
+        if (neg_one_count > 1) {
+            std::string error_msg = fmt::format(
+                "{}.reshape: only one dimension can be -1", ARRAY_TYPE::ARRAY_NAME
+            );
+            throw nb::value_error(error_msg.c_str());
+        }
+
+        if (neg_one_count == 1) {
+            if (_nitems % reshape_size != 0) {
+                std::string error_msg = fmt::format(
+                    "{}.reshape: the size of target array must be unchanged and "
+                    "divisible by the known dimensions",
+                    ARRAY_TYPE::ARRAY_NAME
+                );
+                throw nb::value_error(error_msg.c_str());
+            }
+            new_shape_vec[neg_one_pos] = _nitems / reshape_size;
+        }
+
+        std::size_t reshape_elem_count = fold_shape(new_shape_vec);
+
+        // Validate if total elements match the original array
+        if (_nitems != reshape_elem_count) {
+            std::string error_msg = fmt::format(
+                "{}.reshape: target array number of elements does not match the "
+                "original array",
+                ARRAY_TYPE::ARRAY_NAME
+            );
+            throw nb::value_error(error_msg.c_str());
+        }
+
+        return new_shape_vec;
+    }
+
+    //! Python exported `reshape` method
+    ARRAY_TYPE reshape(nb::tuple shape) const
+    {
+        std::vector<std::size_t> cpp_shape = try_reshape(shape);
+        ARRAY_TYPE result
+            = static_cast<const ARRAY_TYPE*>(this)->create_array(cpp_shape);
+        std::copy_n(this->_data.begin(), _data.size(), result._data.begin());
+
+        return result;
+    }
+
 }; // class APyArray
 
 #endif
diff --git a/src/apyfixedarray.cc b/src/apyfixedarray.cc
index fde63910f..02c96c4ba 100644
--- a/src/apyfixedarray.cc
+++ b/src/apyfixedarray.cc
@@ -1489,15 +1489,6 @@ std::string APyFixedArray::repr() const
     return ss.str();
 }
 
-APyFixedArray APyFixedArray::reshape(nb::tuple new_shape) const
-{
-    std::vector<std::size_t> new_shape_cpp = reshape_from_tuple(new_shape, _nitems);
-    APyFixedArray result = APyFixedArray(new_shape_cpp, _bits, _int_bits);
-    std::copy_n(this->_data.begin(), _data.size(), result._data.begin());
-
-    return result;
-}
-
 APyFixedArray APyFixedArray::flatten() const
 {
     // Reuse the reshape function to flatten the array
diff --git a/src/apyfixedarray.h b/src/apyfixedarray.h
index 2224c4929..8583ec3ce 100644
--- a/src/apyfixedarray.h
+++ b/src/apyfixedarray.h
@@ -251,8 +251,6 @@ class APyFixedArray : public APyArray<mp_limb_t, APyFixedArray> {
     //! Python `__repr__()` function
     std::string repr() const;
 
-    APyFixedArray reshape(nb::tuple new_shape) const;
-
     //! Reshape flatten into 1d shape
     APyFixedArray flatten() const;
 
diff --git a/src/apyfloatarray.cc b/src/apyfloatarray.cc
index 71b5dc4ca..1f88f1d0f 100644
--- a/src/apyfloatarray.cc
+++ b/src/apyfloatarray.cc
@@ -1790,20 +1790,6 @@ std::string APyFloatArray::repr() const
     return ss.str();
 }
 
-APyFloatArray APyFloatArray::reshape(nb::tuple new_shape) const
-{
-    // Argument checking and error handling
-    std::size_t elem_count = ::fold_shape(this->_shape);
-    std::vector<std::size_t> new_shape_vec
-        = ::reshape_from_tuple(new_shape, elem_count);
-
-    std::size_t itemsize = 1;
-    APyFloatArray result = APyFloatArray(new_shape_vec, exp_bits, man_bits, bias);
-    std::copy_n(this->_data.begin(), elem_count * itemsize, result._data.begin());
-
-    return result;
-}
-
 APyFloatArray APyFloatArray::flatten() const
 {
     // Reuse the reshape function to flatten the array
diff --git a/src/apyfloatarray.h b/src/apyfloatarray.h
index 27ad7361e..e6701c6c3 100644
--- a/src/apyfloatarray.h
+++ b/src/apyfloatarray.h
@@ -179,9 +179,6 @@ class APyFloatArray : public APyArray<APyFloatData, APyFloatArray> {
      * *                          Public member functions                           * *
      * ****************************************************************************** */
 
-    //! Reshape into a new shape.
-    APyFloatArray reshape(nb::tuple new_shape) const;
-
     //! Reshape flatten into 1d shape.
     APyFloatArray flatten() const;
 
diff --git a/src/array_utils.h b/src/array_utils.h
index 1f10563bd..918a8f97e 100644
--- a/src/array_utils.h
+++ b/src/array_utils.h
@@ -8,63 +8,12 @@
 #include "nanobind/ndarray.h"
 #include <cstddef>
 #include <nanobind/nanobind.h>
-#include <ostream>
-#include <set>
 #include <unordered_set>
 #include <variant>
 namespace nb = nanobind;
 
 #include <vector>
 
-static APY_INLINE std::vector<std::size_t>
-reshape_from_tuple(nb::tuple new_shape, size_t total_elements)
-{
-    std::vector<std::size_t> new_shape_vec;
-    std::size_t reshape_size = 1;
-    int neg_one_count = 0;
-    int neg_one_pos = -1;
-    int index = 0;
-    for (auto it = new_shape.begin(); it != new_shape.end(); ++it, ++index) {
-        int current_value = nb::cast<int>(*it);
-        if (current_value == -1) {
-            neg_one_count++;
-            neg_one_pos = index;        // store the position of -1
-            new_shape_vec.push_back(1); // temporarily push 1, will be replaced later
-        } else if (current_value < -1) {
-            throw nb::value_error("Negative dimensions less than -1 are not allowed.");
-        } else {
-            std::size_t c_size_t = nb::cast<std::size_t>(*it);
-            new_shape_vec.push_back(c_size_t);
-            reshape_size *= c_size_t;
-        }
-    }
-
-    if (neg_one_count > 1) {
-        throw nb::value_error("Only one dimension can be -1.");
-    }
-
-    if (neg_one_count == 1) {
-        if (total_elements % reshape_size != 0) {
-            throw nb::value_error(
-                "The total size of new array must be unchanged and divisible by the "
-                "known dimensions."
-            );
-        }
-        new_shape_vec[neg_one_pos] = total_elements / reshape_size;
-    }
-
-    std::size_t reshape_elem_count = ::fold_shape(new_shape_vec);
-
-    // Validate if total elements match the original array
-    if (total_elements != reshape_elem_count) {
-        throw nb::value_error(
-            "Total number of elements does not match the original array."
-        );
-    }
-
-    return new_shape_vec;
-}
-
 /**
  * @brief Converts the provided axes into a vector of positive indices.
  *
diff --git a/stubgen.sh b/stubgen.sh
index 100114457..890203f30 100755
--- a/stubgen.sh
+++ b/stubgen.sh
@@ -1,4 +1,5 @@
 #!/usr/bin/env sh
+python3 -c "from nanobind import __version__ as v; print(f'Nanobind version: {v}')"
 python3 -m nanobind.stubgen -P -m apytypes -o lib/apytypes/__init__.pyi
 python3 -m nanobind.stubgen -m apytypes._apytypes -o lib/apytypes/_apytypes.pyi
 python3 -m nanobind.stubgen -m apytypes._array_functions -o lib/apytypes/_array_functions.pyi


Test patch:
diff --git a/lib/test/apyfixedarray/test_methods.py b/lib/test/apyfixedarray/test_methods.py
index 2555c9472..51c71bc23 100644
--- a/lib/test/apyfixedarray/test_methods.py
+++ b/lib/test/apyfixedarray/test_methods.py
@@ -660,13 +660,15 @@ def test_reshape(shape, is_valid, is_invalid, test_neg_one):
     if is_invalid:
         with pytest.raises(
             ValueError,
-            match="Total number of elements does not match the original array.",
+            match=r"APyFixedArray\.reshape: target array number of elements does not",
         ):
             arr.reshape(shape)
     elif test_neg_one:
         with pytest.raises(
             ValueError,
-            match=r"Negative dimensions less than -1 are not allowed.|Only one dimension can be -1.|The total size of new array must be unchanged and divisible by the known dimensions.",
+            match=r"APyFixedArray\.reshape: array dimensions must be greater than|"
+            r"APyFixedArray\.reshape: only one dimension can be -1|"
+            r"APyFixedArray\.reshape: the size of target array must be unchanged and",
         ):
             arr.reshape(shape)
     elif is_valid:
diff --git a/lib/test/apyfloatarray/test_methods.py b/lib/test/apyfloatarray/test_methods.py
index a2aeaa8ed..f787ae090 100644
--- a/lib/test/apyfloatarray/test_methods.py
+++ b/lib/test/apyfloatarray/test_methods.py
@@ -1305,13 +1305,15 @@ def test_reshape(shape, is_valid, is_invalid, test_neg_one):
     if is_invalid:
         with pytest.raises(
             ValueError,
-            match="Total number of elements does not match the original array.",
+            match=r"APyFloatArray\.reshape: target array number of elements does not",
         ):
             arr.reshape(shape)
     elif test_neg_one:
         with pytest.raises(
             ValueError,
-            match=r"Negative dimensions less than -1 are not allowed.|Only one dimension can be -1.|The total size of new array must be unchanged and divisible by the known dimensions.",
+            match=r"APyFloatArray\.reshape: array dimensions must be greater than|"
+            r"APyFloatArray\.reshape: only one dimension can be -1|"
+            r"APyFloatArray\.reshape: the size of target array must be unchanged and",
         ):
             arr.reshape(shape)
     elif is_valid: