ronantakizawa/codereview-bench huggingface dataset:
Repository: numpy/numpy
File: numpy/_core/src/common/npy_hashtable.cpp (C++)

Code before:
    res->buckets = (PyObject **)PyMem_Calloc(4 * (key_len + 1), sizeof(PyObject *));
    if (res->buckets == NULL) {
        PyErr_NoMemory();
        PyMem_Free(res);
        return NULL;
    }

#ifdef Py_GIL_DISABLED
    res->mutex = new std::shared_mutex();
#endif
    return res;
}


NPY_NO_EXPORT void
PyArrayIdentityHash_Dealloc(PyArrayIdentityHash *tb)
{
    PyMem_Free(tb->buckets);
    PyMem_Free(tb);
#ifdef Py_GIL_DISABLED
    delete (std::shared_mutex *)tb->mutex;
#endif
}


static int
_resize_if_necessary(PyArrayIdentityHash *tb)
{
    npy_intp new_size, prev_size = tb->size;
    PyObject **old_table = tb->buckets;
    assert(prev_size > 0);

    if ((tb->nelem + 1) * 2 > prev_size) {
        /* Double in size */
        new_size = prev_size * 2;
    }
    else {
        new_size = prev_size;
        while ((tb->nelem + 8) * 2 < new_size / 2) {
            /*
             * Should possibly be improved.  However, we assume that we
             * almost never shrink.  Still if we do, do not shrink as much
             * as possible to avoid growing right away.
             */
            new_size /= 2;
        }
        assert(new_size >= 4);
    }
    if (new_size == prev_size) {
        return 0;
    }

Review comment (suggestion):
Seems OK to assume that `delete` will never throw an exception.

Code after:
    res->buckets = (PyObject **)PyMem_Calloc(4 * (key_len + 1), sizeof(PyObject *));
    if (res->buckets == NULL) {
        PyErr_NoMemory();
        PyMem_Free(res);
        return NULL;
    }

#ifdef Py_GIL_DISABLED
    res->mutex = new(std::nothrow) std::shared_mutex();
    if (res->mutex == nullptr) {
        PyErr_NoMemory();
        PyMem_Free(res);
        return NULL;
    }
#endif
    return res;
}


NPY_NO_EXPORT void
PyArrayIdentityHash_Dealloc(PyArrayIdentityHash *tb)
{
    PyMem_Free(tb->buckets);
    PyMem_Free(tb);
#ifdef Py_GIL_DISABLED
    delete (std::shared_mutex *)tb->mutex;
#endif
}


static int
_resize_if_necessary(PyArrayIdentityHash *tb)
{
    npy_intp new_size, prev_size = tb->size;
    PyObject **old_table = tb->buckets;
    assert(prev_size > 0);

    if ((tb->nelem + 1) * 2 > prev_size) {
        /* Double in size */
        new_size = prev_size * 2;
    }
    else {
        new_size = prev_size;
        while ((tb->nelem + 8) * 2 < new_size / 2) {
            /*
             * Should possibly be improved.  However, we assume that we
             * almost never shrink.  Still if we do, do not shrink as much
             * as possible to avoid growing right away.
             */
            new_size /= 2;
        }
Repository: numpy/numpy
File: numpy/_core/numeric.py (Python)

Code before:
    array([[0., 1.],
           [0., 1.]])

    >>> np.fromfunction(lambda i, j: i == j, (3, 3), dtype=int)
    array([[ True, False, False],
           [False,  True, False],
           [False, False,  True]])

    >>> np.fromfunction(lambda i, j: i + j, (3, 3), dtype=int)
    array([[0, 1, 2],
           [1, 2, 3],
           [2, 3, 4]])

    """
    if like is not None:
        return _fromfunction_with_like(
                like, function, shape, dtype=dtype, **kwargs)

    args = indices(shape, dtype=dtype)
    return function(*args, **kwargs)


_fromfunction_with_like = array_function_dispatch()(fromfunction)


def _frombuffer(buf, dtype, shape, order, axis):
    if order != 'K':
        return frombuffer(buf, dtype=dtype).reshape(shape, order=order)
    return frombuffer(buf, dtype=dtype).reshape(shape, order='C').transpose(axis)


@set_module('numpy')
def isscalar(element):
    """
    Returns True if the type of `element` is a scalar type.

    Parameters
    ----------
    element : any
        Input argument, can be of any type and shape.

    Returns
    -------
    val : bool
        True if `element` is a scalar type, False if it is not.

    See Also
    --------
    ndim : Get the number of dimensions of an array

    Notes

Review comment (suggestion):
This function must still be able to deal with old pickles.  It is effectively public API.

A test should have existed to find this.  We should use `pickle.dumps(...)` on a current NumPy version, take that string and ensure that `pickle.loads(...)` on it still works (i.e. freeze the pickle in time).

This looks like it breaks existing pickles and we shouldn't.

Code after:
    array([[0., 1.],
           [0., 1.]])

    >>> np.fromfunction(lambda i, j: i == j, (3, 3), dtype=int)
    array([[ True, False, False],
           [False,  True, False],
           [False, False,  True]])

    >>> np.fromfunction(lambda i, j: i + j, (3, 3), dtype=int)
    array([[0, 1, 2],
           [1, 2, 3],
           [2, 3, 4]])

    """
    if like is not None:
        return _fromfunction_with_like(
                like, function, shape, dtype=dtype, **kwargs)

    args = indices(shape, dtype=dtype)
    return function(*args, **kwargs)


_fromfunction_with_like = array_function_dispatch()(fromfunction)


def _frombuffer(buf, dtype, shape, order, axis_order=None):
    if order == 'K' and axis_order is not None:
        return frombuffer(buf, dtype=dtype).reshape(shape, order='C').transpose(axis_order)
    return frombuffer(buf, dtype=dtype).reshape(shape, order=order)


@set_module('numpy')
def isscalar(element):
    """
    Returns True if the type of `element` is a scalar type.

    Parameters
    ----------
    element : any
        Input argument, can be of any type and shape.

    Returns
    -------
    val : bool
        True if `element` is a scalar type, False if it is not.

    See Also
    --------
    ndim : Get the number of dimensions of an array

    Notes
Repository: numpy/numpy
File: numpy/_core/numeric.py (Python)

Code before:
    array([[0., 1.],
           [0., 1.]])

    >>> np.fromfunction(lambda i, j: i == j, (3, 3), dtype=int)
    array([[ True, False, False],
           [False,  True, False],
           [False, False,  True]])

    >>> np.fromfunction(lambda i, j: i + j, (3, 3), dtype=int)
    array([[0, 1, 2],
           [1, 2, 3],
           [2, 3, 4]])

    """
    if like is not None:
        return _fromfunction_with_like(
                like, function, shape, dtype=dtype, **kwargs)

    args = indices(shape, dtype=dtype)
    return function(*args, **kwargs)


_fromfunction_with_like = array_function_dispatch()(fromfunction)


def _frombuffer(buf, dtype, shape, order, axis=None):
    if order == 'K' and axis is not None:
        return frombuffer(buf, dtype=dtype).reshape(shape, order='C').transpose(axis)
    return frombuffer(buf, dtype=dtype).reshape(shape, order=order)


@set_module('numpy')
def isscalar(element):
    """
    Returns True if the type of `element` is a scalar type.

    Parameters
    ----------
    element : any
        Input argument, can be of any type and shape.

    Returns
    -------
    val : bool
        True if `element` is a scalar type, False if it is not.

    See Also
    --------
    ndim : Get the number of dimensions of an array

    Notes

Review comment (suggestion):
Not a big thing, but I might suggest to rename `axis` to `axes_order` or something else that clarifies the use for transposing.

Code after:
    array([[0., 1.],
           [0., 1.]])

    >>> np.fromfunction(lambda i, j: i == j, (3, 3), dtype=int)
    array([[ True, False, False],
           [False,  True, False],
           [False, False,  True]])

    >>> np.fromfunction(lambda i, j: i + j, (3, 3), dtype=int)
    array([[0, 1, 2],
           [1, 2, 3],
           [2, 3, 4]])

    """
    if like is not None:
        return _fromfunction_with_like(
                like, function, shape, dtype=dtype, **kwargs)

    args = indices(shape, dtype=dtype)
    return function(*args, **kwargs)


_fromfunction_with_like = array_function_dispatch()(fromfunction)


def _frombuffer(buf, dtype, shape, order, axis_order=None):
    if order == 'K' and axis_order is not None:
        return frombuffer(buf, dtype=dtype).reshape(shape, order='C').transpose(axis_order)
    return frombuffer(buf, dtype=dtype).reshape(shape, order=order)


@set_module('numpy')
def isscalar(element):
    """
    Returns True if the type of `element` is a scalar type.

    Parameters
    ----------
    element : any
        Input argument, can be of any type and shape.

    Returns
    -------
    val : bool
        True if `element` is a scalar type, False if it is not.

    See Also
    --------
    ndim : Get the number of dimensions of an array

    Notes
Repository: numpy/numpy
File: numpy/_core/src/multiarray/item_selection.c (C)

Code before:
            /* We ensure memory doesn't overlap, so can use memcpy */
            memcpy(ret_data, PyArray_MultiIter_DATA(multi, mi), elsize);
        }
        else {
            char *args[2] = {PyArray_MultiIter_DATA(multi, mi), ret_data};
            if (cast_info.func(&cast_info.context, args, &one,
                                transfer_strides, cast_info.auxdata) < 0) {
                goto fail;
            }
        }
        ret_data += elsize;
        PyArray_MultiIter_NEXT(multi);
    }

    NPY_cast_info_xfree(&cast_info);
    Py_DECREF(multi);
    for (i = 0; i < n; i++) {
        Py_XDECREF(mps[i]);
    }
    Py_DECREF(ap);
    PyDataMem_FREE(mps);
    if (out != NULL && out != obj) {
        Py_INCREF(out);
        int success;
        if (is_newarr) {
            success = PyArray_CopyAnyInto(out, obj);
        }
        else {
            success = PyArray_ResolveWritebackIfCopy(obj);
        }
        if (success < 0) {
            Py_DECREF(obj);
            goto fail;
        }
        Py_DECREF(obj);
        obj = out;
    }
    return (PyObject *)obj;

 fail:
    NPY_cast_info_xfree(&cast_info);
    Py_XDECREF(multi);
    for (i = 0; i < n; i++) {
        Py_XDECREF(mps[i]);
    }
    Py_XDECREF(ap);
    PyDataMem_FREE(mps);
    PyArray_DiscardWritebackIfCopy(obj);
    Py_XDECREF(obj);
    return NULL;
}

Review comment (suggestion):
In theory, you could set the base-object to your allocation above and set to use `WritebackIfCopy` even with your custom allocation.  But, TBH, I think a direct copy is actually clearer (frankly, I am not sure if we couldn't use it in half the places `ResolveWritebackIfCopy` is used ;)).

Code after:
                break;
            }
        }
        if (cast_info.func == NULL) {
            /* We ensure memory doesn't overlap, so can use memcpy */
            memcpy(ret_data, PyArray_MultiIter_DATA(multi, mi), elsize);
        }
        else {
            char *args[2] = {PyArray_MultiIter_DATA(multi, mi), ret_data};
            if (cast_info.func(&cast_info.context, args, &one,
                                transfer_strides, cast_info.auxdata) < 0) {
                goto fail;
            }
        }
        ret_data += elsize;
        PyArray_MultiIter_NEXT(multi);
    }

    NPY_cast_info_xfree(&cast_info);
    Py_DECREF(multi);
    for (i = 0; i < n; i++) {
        Py_XDECREF(mps[i]);
    }
    Py_DECREF(ap);
    PyDataMem_FREE(mps);
    if (copy_existing_out) {
        int res = PyArray_CopyInto(out, obj);
        Py_DECREF(obj);
        if (res < 0) {
            return NULL;
        }
        return Py_NewRef(out);
    }
    return (PyObject *)obj;

 fail:
    NPY_cast_info_xfree(&cast_info);
    Py_XDECREF(multi);
    for (i = 0; i < n; i++) {
        Py_XDECREF(mps[i]);
    }
    Py_XDECREF(ap);
    PyDataMem_FREE(mps);
    PyArray_DiscardWritebackIfCopy(obj);
    Py_XDECREF(obj);
    return NULL;
}

/*
 * These algorithms use special sorting.  They are not called unless the
 * underlying sort function for the type is available.  Note that axis is
Repository: numpy/numpy
File: numpy/testing/_private/utils.py (Python)

Code before:
                 pass_barrier=False, outer_iterations=1,
                 prepare_args=None):
    """Runs a function many times in parallel"""
    for _ in range(outer_iterations):
        with (concurrent.futures.ThreadPoolExecutor(max_workers=max_workers)
              as tpe):
            if prepare_args is None:
                args = []
            else:
                args = prepare_args()
            if pass_barrier:
                barrier = threading.Barrier(max_workers)
                args.append(barrier)
            if pass_count:
                all_args = [(func, i, *args) for i in range(max_workers)]
            else:
                all_args = [(func, *args) for i in range(max_workers)]
            try:
                n_submitted = 0
                futures = []
                for arg in all_args:
                    futures.append(tpe.submit(*arg))
                    n_submitted += 1
            finally:
                if n_submitted < max_workers and pass_barrier:
                    barrier.abort()
            for f in futures:
                f.result()


def get_stringdtype_dtype(na_object, coerce=True):
    # explicit is check for pd_NA because != with pd_NA returns pd_NA
    if na_object is pd_NA or na_object != "unset":
        return np.dtypes.StringDType(na_object=na_object, coerce=coerce)
    else:
        return np.dtypes.StringDType(coerce=coerce)


Review comment (suggestion):
I think this may be clearer to write as:
```
except:  # or BaseExecption if you prefer
    if pass_barrier:
        barrier.abort()
    raise
```
and avoid counting the `n_submitted` above.

(but maybe I missed a subtlety?)

Code after:
                 pass_barrier=False, outer_iterations=1,
                 prepare_args=None):
    """Runs a function many times in parallel"""
    for _ in range(outer_iterations):
        with (concurrent.futures.ThreadPoolExecutor(max_workers=max_workers)
              as tpe):
            if prepare_args is None:
                args = []
            else:
                args = prepare_args()
            if pass_barrier:
                barrier = threading.Barrier(max_workers)
                args.append(barrier)
            if pass_count:
                all_args = [(func, i, *args) for i in range(max_workers)]
            else:
                all_args = [(func, *args) for i in range(max_workers)]
            try:
                futures = []
                for arg in all_args:
                    futures.append(tpe.submit(*arg))
            finally:
                if len(futures) < max_workers and pass_barrier:
                    barrier.abort()
            for f in futures:
                f.result()


def get_stringdtype_dtype(na_object, coerce=True):
    # explicit is check for pd_NA because != with pd_NA returns pd_NA
    if na_object is pd_NA or na_object != "unset":
        return np.dtypes.StringDType(na_object=na_object, coerce=coerce)
    else:
        return np.dtypes.StringDType(coerce=coerce)
Repository: numpy/numpy
File: numpy/lib/tests/test_loadtxt.py (Python)

Code before:
    with open(fname, "w") as fh:
        fh.write("\n".join(data))
    res = np.loadtxt(fname, dtype=str, delimiter=" ", max_rows=nmax)
    os.remove(fname)
    assert len(res) == nmax

@pytest.mark.parametrize("nskip", (0, 10000, 55000, 200000, 300000))
def test_skiprow_exceeding_maxrows_exceeding_chunksize(nskip):
    # tries to read all of the file,
    # or less, equal, greater than _loadtxt_chunksize
    file_length = 400000
    data = np.arange(1,file_length+1).astype(str) + np.array([" a 0.5 1"]*file_length)

    # file-like path
    txt = StringIO("\n".join(data))
    res = np.loadtxt(txt, dtype = 'str', delimiter=" ", skiprows=nskip, max_rows=100005)
    expected_length = min(100005, file_length-nskip)
    assert len(res) == expected_length
    # are the right lines read in res?
    assert np.all(np.equal(np.arange(nskip+1, nskip+1+expected_length).astype(str), res[:,0]))

    # file-obj path
    fd, fname = mkstemp()
    os.close(fd)
    with open(fname, "w") as fh:
        fh.write("\n".join(data))
    res = np.loadtxt(fname, dtype = 'str', delimiter=" ", skiprows=nskip, max_rows=100005)
    expected_length = min(100005, file_length-nskip)
    assert len(res) == expected_length
    # are the right lines read in res?
    assert np.all(np.equal(np.arange(nskip+1, nskip+1+expected_length).astype(str), res[:,0]))


Review comment (suggestion):
Hmmm, we should delete fd again.  (I realize that this is taken from above, although I think there the cleanup isn't missing, it isn't ideal either.)

I don't see that we use this really, but a nice way is to use pytest tmpdir fixture (i.e. just a magic test argument that is filled in by pytest, I think tmpdir is always defined). That way, pytest will take care of cleanup.

Code after:
    with open(fname, "w") as fh:
        fh.write("\n".join(data))
    res = np.loadtxt(fname, dtype=str, delimiter=" ", max_rows=nmax)
    os.remove(fname)
    assert len(res) == nmax

@pytest.mark.parametrize("nskip", (0, 10000, 12345, 50000, 67891, 100000))
def test_skiprow_exceeding_maxrows_exceeding_chunksize(tmpdir, nskip):
    # tries to read a file in chunks by skipping a variable amount of lines,
    # less, equal, greater than max_rows
    file_length = 110000
    data = "\n".join(f"{i} a 0.5 1" for i in range(1, file_length + 1))
    expected_length = min(60000, file_length - nskip)
    expected = np.arange(nskip + 1, nskip + 1 + expected_length).astype(str)

    # file-like path
    txt = StringIO(data)
    res = np.loadtxt(txt, dtype='str', delimiter=" ", skiprows=nskip, max_rows=60000)
    assert len(res) == expected_length
    # are the right lines read in res?
    assert_array_equal(expected, res[:, 0])

    # file-obj path
    tmp_file = tmpdir / "test_data.txt"
    tmp_file.write(data)
    fname = str(tmp_file)
    res = np.loadtxt(fname, dtype='str', delimiter=" ", skiprows=nskip, max_rows=60000)
    assert len(res) == expected_length
    # are the right lines read in res?
    assert_array_equal(expected, res[:, 0])
Repository: numpy/numpy
File: numpy/lib/tests/test_loadtxt.py (Python)

Code before:
        fh.write("\n".join(data))
    res = np.loadtxt(fname, dtype=str, delimiter=" ", max_rows=nmax)
    os.remove(fname)
    assert len(res) == nmax

@pytest.mark.parametrize("nskip", (0, 10000, 12345, 50000, 67891, 100000))
def test_skiprow_exceeding_maxrows_exceeding_chunksize(tmpdir, nskip):
    # tries to read a file in chunks by skipping a variable amount of lines,
    # less, equal, greater than max_rows
    file_length = 110000
    data = "\n".join(f"{i} a 0.5 1" for i in range(1, file_length + 1))
    expected_length = min(60000, file_length - nskip)
    expected = np.arange(nskip + 1, nskip + 1 + expected_length).astype(str)

    # file-like path
    txt = StringIO(data)
    res = np.loadtxt(txt, dtype='str', delimiter=" ", skiprows=nskip, max_rows=60000)
    assert len(res) == expected_length
    # are the right lines read in res?
    assert_array_equal(expected, res[:, 0])

    # file-obj path
    tmp_file = tmpdir / "test_data.txt"
    tmp_file.write(data)
    fname = str(tmp_file)
    res = np.loadtxt(fname, dtype = 'str', delimiter=" ", skiprows=nskip, max_rows=60000)
    assert len(res) == expected_length
    # are the right lines read in res?
    assert_array_equal(expected, res[:, 0])


Review comment (suggestion):
```suggestion
    res = np.loadtxt(fname, dtype='str', delimiter=" ", skiprows=nskip, max_rows=60000)
```

Code after:
        fh.write("\n".join(data))
    res = np.loadtxt(fname, dtype=str, delimiter=" ", max_rows=nmax)
    os.remove(fname)
    assert len(res) == nmax

@pytest.mark.parametrize("nskip", (0, 10000, 12345, 50000, 67891, 100000))
def test_skiprow_exceeding_maxrows_exceeding_chunksize(tmpdir, nskip):
    # tries to read a file in chunks by skipping a variable amount of lines,
    # less, equal, greater than max_rows
    file_length = 110000
    data = "\n".join(f"{i} a 0.5 1" for i in range(1, file_length + 1))
    expected_length = min(60000, file_length - nskip)
    expected = np.arange(nskip + 1, nskip + 1 + expected_length).astype(str)

    # file-like path
    txt = StringIO(data)
    res = np.loadtxt(txt, dtype='str', delimiter=" ", skiprows=nskip, max_rows=60000)
    assert len(res) == expected_length
    # are the right lines read in res?
    assert_array_equal(expected, res[:, 0])

    # file-obj path
    tmp_file = tmpdir / "test_data.txt"
    tmp_file.write(data)
    fname = str(tmp_file)
    res = np.loadtxt(fname, dtype='str', delimiter=" ", skiprows=nskip, max_rows=60000)
    assert len(res) == expected_length
    # are the right lines read in res?
    assert_array_equal(expected, res[:, 0])
Repository: numpy/numpy
File: numpy/linalg/_linalg.pyi (Python)

Code before:
    /,
    *,
    axis: int = ...,
) -> NDArray[unsignedinteger[Any]]: ...
@overload
def cross(
    a: _ArrayLikeInt_co,
    b: _ArrayLikeInt_co,
    /,
    *,
    axis: int = ...,
) -> NDArray[signedinteger[Any]]: ...
@overload
def cross(
    a: _ArrayLikeFloat_co,
    b: _ArrayLikeFloat_co,
    /,
    *,
    axis: int = ...,
) -> NDArray[floating[Any]]: ...
@overload
def cross(
    a: _ArrayLikeComplex_co,
    b: _ArrayLikeComplex_co,
    /,
    *,
    axis: int = ...,
) -> NDArray[complexfloating[Any, Any]]: ...

@overload
def matmul(
    x1: _ArrayLikeInt_co,
    x2: _ArrayLikeInt_co,
) -> NDArray[signedinteger[Any]]: ...
@overload
def matmul(
    x1: _ArrayLikeUInt_co,
    x2: _ArrayLikeUInt_co,
) -> NDArray[unsignedinteger[Any]]: ...
@overload
def matmul(
    x1: _ArrayLikeFloat_co,
    x2: _ArrayLikeFloat_co,
) -> NDArray[floating[Any]]: ...
@overload
def matmul(
    x1: _ArrayLikeComplex_co,
    x2: _ArrayLikeComplex_co,
) -> NDArray[complexfloating[Any, Any]]: ...


Review comment (suggestion):
the `astype` case is clear, but for `cross`?

The below runs just fine for me locally on latest `main`. Is there some reason the type hinting signature should enforce `/` before the runtime does?

```python
import numpy as np
x = [1, 2, 3]
y = [4, 5, 6]
print(np.cross(a=x, b=y))
```

Code after:
    /,
    *,
    axis: int = ...,
) -> NDArray[unsignedinteger[Any]]: ...
@overload
def cross(
    x1: _ArrayLikeInt_co,
    x2: _ArrayLikeInt_co,
    /,
    *,
    axis: int = ...,
) -> NDArray[signedinteger[Any]]: ...
@overload
def cross(
    x1: _ArrayLikeFloat_co,
    x2: _ArrayLikeFloat_co,
    /,
    *,
    axis: int = ...,
) -> NDArray[floating[Any]]: ...
@overload
def cross(
    x1: _ArrayLikeComplex_co,
    x2: _ArrayLikeComplex_co,
    /,
    *,
    axis: int = ...,
) -> NDArray[complexfloating[Any, Any]]: ...

@overload
def matmul(
    x1: _ArrayLikeInt_co,
    x2: _ArrayLikeInt_co,
) -> NDArray[signedinteger[Any]]: ...
@overload
def matmul(
    x1: _ArrayLikeUInt_co,
    x2: _ArrayLikeUInt_co,
) -> NDArray[unsignedinteger[Any]]: ...
@overload
def matmul(
    x1: _ArrayLikeFloat_co,
    x2: _ArrayLikeFloat_co,
) -> NDArray[floating[Any]]: ...
@overload
def matmul(
    x1: _ArrayLikeComplex_co,
    x2: _ArrayLikeComplex_co,
) -> NDArray[complexfloating[Any, Any]]: ...