Apply suggestions from code review

numpy · mattip · Dec 17, 2024 · Nov 28, 2024 · Nov 30, 2024 · Dec 2, 2024
commit facd23702fa3aa2aff5638483c01ebf1a929bb1a
diff --git a/numpy/_core/src/multiarray/nditer_api.c b/numpy/_core/src/multiarray/nditer_api.c
@@ -808,7 +808,6 @@ NpyIter_IsFirstVisit(NpyIter *iter, int iop)
      * We only need to check the outer level of this two-level loop,
      * because of the requirement that EXTERNAL_LOOP be enabled.
      */
-     // TODO: Do I need to change this?  Chance is no, so long REDUCE_OUTERSTRIDES still makes sense!
     if (itflags&NPY_ITFLAG_BUFFER) {
         NpyIter_BufferData *bufferdata = NIT_BUFFERDATA(iter);
         /* The outer reduce loop */

diff --git a/numpy/_core/src/multiarray/nditer_constr.c b/numpy/_core/src/multiarray/nditer_constr.c
@@ -2018,7 +2018,6 @@ npyiter_find_buffering_setup(NpyIter *iter)
     /*
      * Once a reduce operand reaches a ==0/!=0 stride flip, this dimension
      * becomes the outer reduce dimension.
-     * We may continue growing, but only if strides align for any such operand.
      */
     int outer_reduce_dim = 0;
 
@@ -2051,10 +2050,7 @@ npyiter_find_buffering_setup(NpyIter *iter)
         for (iop = 0; iop < nop; iop++) {
             /* Check that we set things up nicely (if shape is ever 1) */
             assert((axisdata->shape == 1) ? (prev_strides[iop] == strides[iop]) : 1);
-            /*
-             * Best case: the strides collaps for this operand, all fine.
-             * Keep track at which single-stride or outer dims we are.
-             */
+            /*  Best case: the strides collapse for this operand. */
             if (prev_strides[iop] * prev_shape == strides[iop]) {
                 if (op_single_stride_dims[iop] == idim) {
                     op_single_stride_dims[iop] += 1;
@@ -2095,7 +2091,7 @@ npyiter_find_buffering_setup(NpyIter *iter)
             assert(!op_reduce_outer_dim[iop] || op_reduce_outer_dim[iop] == outer_reduce_dim);
         }
         if (iop != nop) {
-            /* Including this dimension is invalid due to a reduction. */
+            /* Including this dimension was invalid due to a reduction. */
             break;
         }
 
@@ -2282,12 +2278,6 @@ npyiter_find_buffering_setup(NpyIter *iter)
             best_size = best_coresize * (maximum_size / best_coresize);
         }
     }
-    /*
-     * Set the buffersize to either the:
-     * - the largest we amount trivially iterate (no buffering!).
-     * - the largest multiple of the coresize that is smaller than the
-     *   requested/default buffersize.
-     */
     NIT_BUFFERDATA(iter)->buffersize = best_size;
     /* Core size is 0 (unless the user applies a range explicitly). */
     NIT_BUFFERDATA(iter)->coreoffset = 0;

diff --git a/numpy/_core/src/multiarray/nditer_impl.h b/numpy/_core/src/multiarray/nditer_impl.h
@@ -106,7 +106,6 @@
 #define NPY_ITFLAG_REDUCE       (1 << 12)
 /* Reduce iteration doesn't need to recalculate reduce loops next time */
 #define NPY_ITFLAG_REUSE_REDUCE_LOOPS (1 << 13)
-
 /*
  * Offset of (combined) ArrayMethod flags for all transfer functions.
  * For now, we use the top 8 bits.

diff --git a/numpy/_core/src/umath/ufunc_object.c b/numpy/_core/src/umath/ufunc_object.c
@@ -2493,6 +2493,7 @@ reduce_loop(PyArrayMethod_Context *context,
             dataptrs_copy[3] = dataptrs[2];
             strides_copy[3] = strides[2];
         }
+
         retval = strided_loop(context,
                 dataptrs_copy, countptr, strides_copy, auxdata);
         if (retval < 0) {

diff --git a/numpy/_core/tests/test_einsum.py b/numpy/_core/tests/test_einsum.py
@@ -5,8 +5,7 @@
 import numpy as np
 from numpy.testing import (
     assert_, assert_equal, assert_array_equal, assert_almost_equal,
-    assert_raises, suppress_warnings, assert_raises_regex, assert_allclose,
-    assert_array_almost_equal_nulp
+    assert_raises, suppress_warnings, assert_raises_regex, assert_allclose
     )
 
 # Setup for optimize einsum

diff --git a/numpy/_core/tests/test_nditer.py b/numpy/_core/tests/test_nditer.py
@@ -2681,8 +2681,8 @@ def test_iter_buffering_reduction():
     it = np.nditer([p, None],
             ['delay_bufalloc', 'reduce_ok', 'buffered', 'external_loop'],
             [['readonly'], ['readwrite', 'allocate']],
-            op_axes=[[-1, 0], [1, 0]],
-            itershape=(2, 2), op_dtypes=["float64", "int64"])
+            op_axes=[[-1, 0], [-1, -1]],
+            itershape=(2, 2))
     with it:
         it.operands[1].fill(0)
         it.reset()