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

Code before:
    PyObject *res = NULL;
    if (!PyArg_ParseTuple(args, "O&", PyArray_Converter, &self))
        return NULL;
    // Making sure the DECREF is called when the function returns, with
    // or w/o an exception
    auto self_decref = finally([&]() { Py_XDECREF(self); });

    try {
        /* Handle zero-sized arrays specially */
        if (PyArray_SIZE(self) == 0) {
            return PyArray_NewLikeArray(
                self,
                NPY_ANYORDER,
                NULL, // descr (use prototype's descr)
                1 // subok
            );
        }

        auto type = PyArray_TYPE(self);
        // we only support data types present in our unique_funcs map
        if (unique_funcs.find(type) == unique_funcs.end()) {
            // Grab GIL before raising an exception
            NPY_ALLOW_C_API;
            PyErr_SetString(PyExc_NotImplementedError, "Unsupported dtype");
            NPY_DISABLE_C_API;
            return NULL;
        }

        res = unique_funcs[type](self);
        return res;
    }
    catch (const std::bad_alloc &e) {
        // Grab GIL before raising an exception
        NPY_ALLOW_C_API;
        PyErr_NoMemory();
        NPY_DISABLE_C_API;
        return NULL;
    }
    catch (const std::exception &e) {
        // Grab GIL before raising an exception
        NPY_ALLOW_C_API;
        PyErr_SetString(PyExc_RuntimeError, e.what());
        NPY_DISABLE_C_API;
        return NULL;
    }
}


Review comment (performance):
```suggestion
            return Py_RETURN_NOTIMPLEMENTED;
```
slight misunderstanding: Since this is an internal functionality, I think returning `NotImplemented` is the clearest, not raising the error :).  (raising an error can a bit slow, although it may have improved)

Code after:
 * type is unsupported or `NULL` with an error set.
 */
extern "C" NPY_NO_EXPORT PyObject *
array__unique_hash(PyObject *NPY_UNUSED(module), PyObject *arr_obj)
{
    if (!PyArray_Check(arr_obj)) {
        PyErr_SetString(PyExc_TypeError,
                "_unique_hash() requires a NumPy array input.");
        return NULL;
    }
    PyArrayObject *arr = (PyArrayObject *)arr_obj;

    try {
        auto type = PyArray_TYPE(arr);
        // we only support data types present in our unique_funcs map
        if (unique_funcs.find(type) == unique_funcs.end()) {
            Py_RETURN_NOTIMPLEMENTED;
        }

        return unique_funcs[type](arr);
    }
    catch (const std::bad_alloc &e) {
        PyErr_NoMemory();
        return NULL;
    }
    catch (const std::exception &e) {
        PyErr_SetString(PyExc_RuntimeError, e.what());
        return NULL;
    }
}
Repository: numpy/numpy
File: numpy/_core/src/multiarray/unique.cpp (C++)

Code before:
    PyThreadState *_save;
    _save = PyEval_SaveThread();
    // Making sure the GIL is re-acquired when the function returns, with
    // or w/o an exception
    auto grab_gil = finally([&]() { PyEval_RestoreThread(_save); });
    // first we put the data in a hash map
    do {
        char* data = *dataptr;
        npy_intp stride = *strideptr;
        npy_intp count = *innersizeptr;

        while (count--) {
            hashset.insert(*((T *) data));
            data += stride;
        }
    } while(iternext(iter));

    npy_intp length = hashset.size();

    NPY_ALLOW_C_API;
    PyArray_Descr *descr = PyArray_DESCR(self);
    Py_INCREF(descr);
    PyObject *res_obj = PyArray_NewFromDescr(
        &PyArray_Type,
        descr,
        1, // ndim
        &length, // shape
        NULL, // strides
        NULL, // data
        // This flag is needed to be able to call .sort on it.
        NPY_ARRAY_WRITEABLE, // flags
        NULL // obj
    );
    NPY_DISABLE_C_API;

    if (res_obj == NULL) {
        return NULL;
    }

    // then we iterate through the map's keys to get the unique values
    T* data = (T *)PyArray_DATA((PyArrayObject *)res_obj);
    auto it = hashset.begin();
    size_t i = 0;
    for (; it != hashset.end(); it++, i++) {
        data[i] = *it;
    }

    return res_obj;
}



Review comment (suggestion):
If there was a C++ exception after this, we would leak it... But, I don't think that is possible, so let's not worry about it.

Code after:
    PyThreadState *_save;
    _save = PyEval_SaveThread();
    // Making sure the GIL is re-acquired when the function returns, with
    // or w/o an exception
    auto grab_gil = finally([&]() { PyEval_RestoreThread(_save); });
    // first we put the data in a hash map

    if (NpyIter_GetIterSize(iter) > 0) {
        do {
            char* data = *dataptr;
            npy_intp stride = *strideptr;
            npy_intp count = *innersizeptr;

            while (count--) {
                hashset.insert(*((T *) data));
                data += stride;
            }
        } while (iternext(iter));
    }

    npy_intp length = hashset.size();

    NPY_ALLOW_C_API;
    PyArray_Descr *descr = PyArray_DESCR(self);
    Py_INCREF(descr);
    PyObject *res_obj = PyArray_NewFromDescr(
        &PyArray_Type,
        descr,
        1, // ndim
        &length, // shape
        NULL, // strides
        NULL, // data
        // This flag is needed to be able to call .sort on it.
        NPY_ARRAY_WRITEABLE, // flags
        NULL // obj
    );
    NPY_DISABLE_C_API;

    if (res_obj == NULL) {
        return NULL;
    }

    // then we iterate through the map's keys to get the unique values
    T* data = (T *)PyArray_DATA((PyArrayObject *)res_obj);
    auto it = hashset.begin();
    size_t i = 0;
    for (; it != hashset.end(); it++, i++) {
        data[i] = *it;
    }

    return res_obj;
Repository: numpy/numpy
File: .spin/cmds.py (Python)

Code before:
    -----

    \b
     - This command only runs doctests and skips everything under tests/
     - This command only doctests public objects: those which are accessible
       from the top-level `__init__.py` file.

    """  # noqa: E501
    if (not pytest_args):
        pytest_args = ('numpy',)

    if (n_jobs != "1") and ('-n' not in pytest_args):
        pytest_args = ('-n', str(n_jobs)) + pytest_args

    if verbose:
        pytest_args = ('-v',) + pytest_args


    doctest_args = (
        # ignores are for things fail doctest collection (optionals etc)
        '--ignore=numpy/distutils',
        '--ignore=numpy/_core/cversions.py',
        '--ignore=numpy/_pyinstaller',
        '--ignore=numpy/random/_examples',
        '--ignore=numpy/compat',
        '--ignore=numpy/f2py/_backends/_distutils.py',
        # turn doctesting on:
        '--doctest-modules',
        '--doctest-collect=api'
    )

    pytest_args = pytest_args + doctest_args

    ctx.params['pytest_args'] = pytest_args

    for extra_param in ('n_jobs', 'verbose'):
        del ctx.params[extra_param]

    ctx.forward(meson.test)



# From scipy: benchmarks/benchmarks/common.py
def _set_mem_rlimit(max_mem=None):
    """
    Set address space rlimit
    """
    import resource
    import psutil

    mem = psutil.virtual_memory()

Review comment (suggestion):
Can't these be part of `conftest.py` or `pyproject.toml`?  It's nice to keep spin invokations simple.

Code after:
    -----

    \b
     - This command only runs doctests and skips everything under tests/
     - This command only doctests public objects: those which are accessible
       from the top-level `__init__.py` file.

    """  # noqa: E501
    if (not pytest_args):
        pytest_args = ('numpy',)

    if (n_jobs != "1") and ('-n' not in pytest_args):
        pytest_args = ('-n', str(n_jobs)) + pytest_args

    if verbose:
        pytest_args = ('-v',) + pytest_args

    # turn doctesting on:
    doctest_args = (
        '--doctest-modules',
        '--doctest-collect=api'
    )

    pytest_args = pytest_args + doctest_args

    ctx.params['pytest_args'] = pytest_args

    for extra_param in ('n_jobs', 'verbose'):
        del ctx.params[extra_param]

    ctx.forward(meson.test)



# From scipy: benchmarks/benchmarks/common.py
def _set_mem_rlimit(max_mem=None):
    """
    Set address space rlimit
    """
    import resource
    import psutil

    mem = psutil.virtual_memory()

    if max_mem is None:
        max_mem = int(mem.total * 0.7)
    cur_limit = resource.getrlimit(resource.RLIMIT_AS)
    if cur_limit[0] > 0:
        max_mem = min(max_mem, cur_limit[0])

    try:
Repository: numpy/numpy
File: .spin/cmds.py (Python)

Code before:

    if verbose:
        pytest_args = ('-v',) + pytest_args

    ctx.params['pytest_args'] = pytest_args

    for extra_param in ('markexpr', 'n_jobs', 'tests', 'verbose'):
        del ctx.params[extra_param]
    ctx.forward(meson.test)


@click.command()
@click.argument("pytest_args", nargs=-1)
@click.option(
    "-j",
    "n_jobs",
    metavar='N_JOBS',
    default="1",
    help=("Number of parallel jobs for testing. "
          "Can be set to `auto` to use all cores.")
)
@click.option(
    '--verbose', '-v', is_flag=True, default=False
)
@click.pass_context
def smoke_docs(ctx, pytest_args, n_jobs, verbose, *args, **kwargs):
    """🔧 Run doctests of objects in the public API.

    PYTEST_ARGS are passed through directly to pytest, e.g.:

      spin smoke-docs -- --pdb

    To run tests on a directory:

     \b
     spin smoke-docs numpy/linalg

    To report the durations of the N slowest doctests:

      spin smoke-docs -- --durations=N

    To run doctests that match a given pattern:

     \b
     spin smoke-docs -- -k "slogdet"
     spin smoke-docs numpy/linalg -- -k "det and not slogdet"

    \b
    Note:
    -----


Review comment (suggestion):
FWIW, I agree with Aaron.  To me "smoke" would probably mean quick/non-comprehensive tests (not sure if that is the case).
But more importantly, I think "smoke" isn't very obvious for testing, `doctest` seems better (or `doc(s)test(s)`!?).

Code after:

    if verbose:
        pytest_args = ('-v',) + pytest_args

    ctx.params['pytest_args'] = pytest_args

    for extra_param in ('markexpr', 'n_jobs', 'tests', 'verbose'):
        del ctx.params[extra_param]
    ctx.forward(meson.test)


@click.command()
@click.argument("pytest_args", nargs=-1)
@click.option(
    "-j",
    "n_jobs",
    metavar='N_JOBS',
    default="1",
    help=("Number of parallel jobs for testing. "
          "Can be set to `auto` to use all cores.")
)
@click.option(
    '--verbose', '-v', is_flag=True, default=False
)
@click.pass_context
def check_docs(ctx, pytest_args, n_jobs, verbose, *args, **kwargs):
    """🔧 Run doctests of objects in the public API.

    PYTEST_ARGS are passed through directly to pytest, e.g.:

      spin check-docs -- --pdb

    To run tests on a directory:

     \b
     spin check-docs numpy/linalg

    To report the durations of the N slowest doctests:

      spin check-docs -- --durations=N

    To run doctests that match a given pattern:

     \b
     spin check-docs -- -k "slogdet"
     spin check-docs numpy/linalg -- -k "det and not slogdet"

    \b
    Note:
    -----
Repository: numpy/numpy
File: numpy/conftest.py (Python)

Code before:
    # find and check doctests under this context manager
    dt_config.user_context_mgr = warnings_errors_and_rng

    # numpy specific tweaks from refguide-check
    dt_config.rndm_markers.add('#uninitialized')
    dt_config.rndm_markers.add('# uninitialized')

    import doctest
    dt_config.optionflags = doctest.NORMALIZE_WHITESPACE | doctest.ELLIPSIS

    # recognize the StringDType repr
    dt_config.check_namespace['StringDType'] = numpy.dtypes.StringDType

    # temporary skips
    dt_config.skiplist = set([
        'numpy.savez',    # unclosed file
        'numpy.matlib.savez',
        'numpy.__array_namespace_info__',
        'numpy.matlib.__array_namespace_info__',
    ])

    # xfail problematic tutorials
    dt_config.pytest_extra_xfail = {
        'how-to-verify-bug.rst': '',
        'c-info.ufunc-tutorial.rst': '',
        'basics.interoperability.rst': 'needs pandas',
        'basics.dispatch.rst': 'errors out in /testing/overrides.py',
        'basics.subclassing.rst': '.. testcode:: admonitions not understood'
    }



Review comment (suggestion):
This is an example of something that `pytest-doctestplus` probably manages (I am not 100% sure for rst docs).

Code after:
    # find and check doctests under this context manager
    dt_config.user_context_mgr = warnings_errors_and_rng

    # numpy specific tweaks from refguide-check
    dt_config.rndm_markers.add('#uninitialized')
    dt_config.rndm_markers.add('# uninitialized')

    import doctest
    dt_config.optionflags = doctest.NORMALIZE_WHITESPACE | doctest.ELLIPSIS

    # recognize the StringDType repr
    dt_config.check_namespace['StringDType'] = numpy.dtypes.StringDType

    # temporary skips
    dt_config.skiplist = set([
        'numpy.savez',    # unclosed file
        'numpy.matlib.savez',
        'numpy.__array_namespace_info__',
        'numpy.matlib.__array_namespace_info__',
    ])

    # xfail problematic tutorials
    dt_config.pytest_extra_xfail = {
        'how-to-verify-bug.rst': '',
        'c-info.ufunc-tutorial.rst': '',
        'basics.interoperability.rst': 'needs pandas',
        'basics.dispatch.rst': 'errors out in /testing/overrides.py',
        'basics.subclassing.rst': '.. testcode:: admonitions not understood'
    }

    # ignores are for things fail doctest collection (optionals etc)
    dt_config.pytest_extra_ignore = [
        'numpy/distutils',
        'numpy/_core/cversions.py',
        'numpy/_pyinstaller',
        'numpy/random/_examples',
        'numpy/compat',
        'numpy/f2py/_backends/_distutils.py',
    ]
Repository: numpy/numpy
File: numpy/_core/_add_newdocs.py (Python)

Code before:
add_newdoc('numpy._core.multiarray', 'StringDType',
    """
    StringDType(*, na_object=np._NoValue, coerce=True)

    Create a StringDType instance.

    StringDType can be used to store UTF-8 encoded variable-width strings in
    a NumPy array.

    Parameters
    ----------
    na_object : object, optional
        Object used to represent missing data. If unset, the array will not
        use a missing data sentinel.
    coerce : bool, optional
        Whether or not items in an array-like passed to an array creation
        function that are neither a str or str subtype should be coerced to
        str. Defaults to True. If set to False, creating a StringDType
        array from an array-like containing entries that are not already
        strings will raise an error.

    Examples
    --------

    >>> from numpy.dtypes import StringDType
    >>> np.array(["hello", "world"], dtype=StringDType())
    array(["hello", "world"], dtype=StringDType())

    >>> arr = np.array(["hello", None, "world"],
                       dtype=StringDType(na_object=None))
    >>> arr
    array(["hello", None, "world", dtype=StringDType(na_object=None))
    >>> arr[1] is None
    True

    >>> arr = np.array(["hello", np.nan, "world"],
                       dtype=StringDType(na_object=np.nan))
    >>> np.isnan(arr)
    array([False, True, False])

    >>> np.array([1.2, object(), "hello world"],
                 dtype=StringDType(coerce=True))
    ValueError: StringDType only allows string data when string coercion
    is disabled.

    >>> np.array(["hello", "world"], dtype=StringDType(coerce=True))
    array(["hello", "world"], dtype=StringDType(coerce=True))
    """)


Review comment (suggestion):
 This is taken over from the initial version, but should this not be `coerce=False`? – see also https://github.com/numpy/numpy/blob/a1f2d582f84878b6c67bd641fa671a4cf868fe8c/doc/source/user/basics.strings.rst?plain=1#L185-L188

Code after:
        function that are neither a str or str subtype should be coerced to
        str. Defaults to True. If set to False, creating a StringDType
        array from an array-like containing entries that are not already
        strings will raise an error.

    Examples
    --------

    >>> from numpy.dtypes import StringDType
    >>> np.array(["hello", "world"], dtype=StringDType())
    array(["hello", "world"], dtype=StringDType())

    >>> arr = np.array(["hello", None, "world"],
    ...                dtype=StringDType(na_object=None))
    >>> arr
    array(["hello", None, "world"], dtype=StringDType(na_object=None))
    >>> arr[1] is None
    True

    >>> arr = np.array(["hello", np.nan, "world"],
    ...                dtype=StringDType(na_object=np.nan))
    >>> np.isnan(arr)
    array([False, True, False])

    >>> np.array([1.2, object(), "hello world"],
    ...          dtype=StringDType(coerce=True))
    ValueError: StringDType only allows string data when string coercion
    is disabled.

    >>> np.array(["hello", "world"], dtype=StringDType(coerce=True))
    array(["hello", "world"], dtype=StringDType(coerce=True))
    """)
Repository: numpy/numpy
File: benchmarks/benchmarks/bench_array_coercion.py (Python)

Code before:
        try:
            np.array(array_like, ndmin="not-integer")
        except TypeError:
            pass

    def time_array(self, array_like):
        np.array(array_like)

    def time_array_dtype_not_kwargs(self, array_like):
        np.array(array_like, self.int64)

    def time_array_no_copy(self, array_like):
        np.array(array_like, copy=None)

    def time_array_subok(self, array_like):
        np.array(array_like, subok=True)

    def time_array_all_kwargs(self, array_like):
        np.array(array_like, dtype=self.int64, copy=None, order="F",
                 subok=False, ndmin=2)

    def time_asarray(self, array_like):
        np.asarray(array_like)

    def time_asarray_dtype(self, array_like):
        np.asarray(array_like, dtype=self.int64, order="F")

    def time_asanyarray(self, array_like):
        np.asanyarray(array_like)

    def time_asanyarray_dtype(self, array_like):
        np.asanyarray(array_like, dtype=self.int64, order="F")

    def time_ascontiguousarray(self, array_like):
        np.ascontiguousarray(array_like)


Review comment (suggestion):
Two versions of function `time_asarray_dtype` here, with/without `order="F"` argument.

Code after:
        try:
            np.array(array_like, ndmin="not-integer")
        except TypeError:
            pass

    def time_array(self, array_like):
        np.array(array_like)

    def time_array_dtype_not_kwargs(self, array_like):
        np.array(array_like, self.int64)

    def time_array_no_copy(self, array_like):
        np.array(array_like, copy=None)

    def time_array_subok(self, array_like):
        np.array(array_like, subok=True)

    def time_array_all_kwargs(self, array_like):
        np.array(array_like, dtype=self.int64, copy=None, order="F",
                 subok=False, ndmin=2)

    def time_asarray(self, array_like):
        np.asarray(array_like)

    def time_asarray_dtype(self, array_like):
        np.asarray(array_like, dtype=self.int64)

    def time_asarray_dtype_order(self, array_like):
        np.asarray(array_like, dtype=self.int64, order="F")

    def time_asanyarray(self, array_like):
        np.asanyarray(array_like)

    def time_asanyarray_dtype(self, array_like):
        np.asanyarray(array_like, dtype=self.int64)

    def time_asanyarray_dtype_order(self, array_like):
        np.asanyarray(array_like, dtype=self.int64, order="F")

    def time_ascontiguousarray(self, array_like):
        np.ascontiguousarray(array_like)
Repository: numpy/numpy
File: numpy/_core/src/common/npy_hashtable.cpp (C++)

Code before:
}


NPY_NO_EXPORT PyArrayIdentityHash *
PyArrayIdentityHash_New(int key_len)
{
    PyArrayIdentityHash *res = (PyArrayIdentityHash *)PyMem_Malloc(sizeof(PyArrayIdentityHash));
    if (res == NULL) {
        PyErr_NoMemory();
        return NULL;
    }

    assert(key_len > 0);
    res->key_len = key_len;
    res->size = 4;  /* Start with a size of 4 */
    res->nelem = 0;

    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 */

Review comment (suggestion):
I'm using a heap allocation because otherwise I'd need to convert `ufunc_object.c` - which includes `npy_hashtable.h` and sees the layout of `PyArrayIdentityHash` - to C++ and that seemed like too much to take on.

Code after:
}


NPY_NO_EXPORT PyArrayIdentityHash *
PyArrayIdentityHash_New(int key_len)
{
    PyArrayIdentityHash *res = (PyArrayIdentityHash *)PyMem_Malloc(sizeof(PyArrayIdentityHash));
    if (res == NULL) {
        PyErr_NoMemory();
        return NULL;
    }

    assert(key_len > 0);
    res->key_len = key_len;
    res->size = 4;  /* Start with a size of 4 */
    res->nelem = 0;

    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;