Skip to content

BUG: groupby and agg on read-only array gives ValueError: buffer source array is read-only #36061

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 4, 2020
Merged

BUG: groupby and agg on read-only array gives ValueError: buffer source array is read-only #36061

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd5972b
fix groupby for read-only fused values
jeet-parekh Sep 2, 2020
2c166e5
minor changes, tests, and whatsnew entry
jeet-parekh Sep 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 22 additions & 14 deletions pandas/_libs/groupby.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ def group_cumprod_float64(float64_t[:, :] out,
@cython.boundscheck(False)
@cython.wraparound(False)
def group_cumsum(numeric[:, :] out,
numeric[:, :] values,
ndarray[numeric, ndim=2] values,
const int64_t[:] labels,
int ngroups,
is_datetimelike,
Expand Down Expand Up @@ -472,7 +472,7 @@ ctypedef fused complexfloating_t:
@cython.boundscheck(False)
def _group_add(complexfloating_t[:, :] out,
int64_t[:] counts,
complexfloating_t[:, :] values,
ndarray[complexfloating_t, ndim=2] values,
const int64_t[:] labels,
Py_ssize_t min_count=0):
"""
Expand All @@ -483,8 +483,10 @@ def _group_add(complexfloating_t[:, :] out,
complexfloating_t val, count
complexfloating_t[:, :] sumx
int64_t[:, :] nobs
Py_ssize_t len_values = len(values)
Py_ssize_t len_labels = len(labels)

if len(values) != len(labels):
if len_values != len_labels:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think elsewhere we get around this be using values.shape[0] instead of len. IIRC this is something we wont need to do anymore in cython3

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are indeed a few cases of

    # TODO(cython 3.0):
    # Instead of `labels.shape[0]` use `len(labels)`
    if not len(values) == labels.shape[0]:

in this file. But so if we need to update it anyways after cython 3.0, it doesn't maybe matter that much.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i think ok for now

raise ValueError("len(index) != len(labels)")

nobs = np.zeros((<object>out).shape, dtype=np.int64)
Expand Down Expand Up @@ -530,7 +532,7 @@ group_add_complex128 = _group_add['double complex']
@cython.boundscheck(False)
def _group_prod(floating[:, :] out,
int64_t[:] counts,
floating[:, :] values,
ndarray[floating, ndim=2] values,
const int64_t[:] labels,
Py_ssize_t min_count=0):
"""
Expand All @@ -541,8 +543,10 @@ def _group_prod(floating[:, :] out,
floating val, count
floating[:, :] prodx
int64_t[:, :] nobs
Py_ssize_t len_values = len(values)
Py_ssize_t len_labels = len(labels)

if not len(values) == len(labels):
if len_values != len_labels:
raise ValueError("len(index) != len(labels)")

nobs = np.zeros((<object>out).shape, dtype=np.int64)
Expand Down Expand Up @@ -582,7 +586,7 @@ group_prod_float64 = _group_prod['double']
@cython.cdivision(True)
def _group_var(floating[:, :] out,
int64_t[:] counts,
floating[:, :] values,
ndarray[floating, ndim=2] values,
const int64_t[:] labels,
Py_ssize_t min_count=-1,
int64_t ddof=1):
Expand All @@ -591,10 +595,12 @@ def _group_var(floating[:, :] out,
floating val, ct, oldmean
floating[:, :] mean
int64_t[:, :] nobs
Py_ssize_t len_values = len(values)
Py_ssize_t len_labels = len(labels)

assert min_count == -1, "'min_count' only used in add and prod"

if not len(values) == len(labels):
if len_values != len_labels:
raise ValueError("len(index) != len(labels)")

nobs = np.zeros((<object>out).shape, dtype=np.int64)
Expand Down Expand Up @@ -639,18 +645,20 @@ group_var_float64 = _group_var['double']
@cython.boundscheck(False)
def _group_mean(floating[:, :] out,
int64_t[:] counts,
floating[:, :] values,
ndarray[floating, ndim=2] values,
const int64_t[:] labels,
Py_ssize_t min_count=-1):
cdef:
Py_ssize_t i, j, N, K, lab, ncounts = len(counts)
floating val, count
floating[:, :] sumx
int64_t[:, :] nobs
Py_ssize_t len_values = len(values)
Py_ssize_t len_labels = len(labels)

assert min_count == -1, "'min_count' only used in add and prod"

if not len(values) == len(labels):
if len_values != len_labels:
raise ValueError("len(index) != len(labels)")

nobs = np.zeros((<object>out).shape, dtype=np.int64)
Expand Down Expand Up @@ -689,7 +697,7 @@ group_mean_float64 = _group_mean['double']
@cython.boundscheck(False)
def _group_ohlc(floating[:, :] out,
int64_t[:] counts,
floating[:, :] values,
ndarray[floating, ndim=2] values,
const int64_t[:] labels,
Py_ssize_t min_count=-1):
"""
Expand Down Expand Up @@ -740,7 +748,7 @@ group_ohlc_float64 = _group_ohlc['double']
@cython.boundscheck(False)
@cython.wraparound(False)
def group_quantile(ndarray[float64_t] out,
numeric[:] values,
ndarray[numeric, ndim=1] values,
ndarray[int64_t] labels,
ndarray[uint8_t] mask,
float64_t q,
Expand Down Expand Up @@ -1072,7 +1080,7 @@ def group_nth(rank_t[:, :] out,
@cython.boundscheck(False)
@cython.wraparound(False)
def group_rank(float64_t[:, :] out,
rank_t[:, :] values,
ndarray[rank_t, ndim=2] values,
const int64_t[:] labels,
int ngroups,
bint is_datetimelike, object ties_method="average",
Expand Down Expand Up @@ -1424,7 +1432,7 @@ def group_min(groupby_t[:, :] out,
@cython.boundscheck(False)
@cython.wraparound(False)
def group_cummin(groupby_t[:, :] out,
groupby_t[:, :] values,
ndarray[groupby_t, ndim=2] values,
const int64_t[:] labels,
int ngroups,
bint is_datetimelike):
Expand Down Expand Up @@ -1484,7 +1492,7 @@ def group_cummin(groupby_t[:, :] out,
@cython.boundscheck(False)
@cython.wraparound(False)
def group_cummax(groupby_t[:, :] out,
groupby_t[:, :] values,
ndarray[groupby_t, ndim=2] values,
const int64_t[:] labels,
int ngroups,
bint is_datetimelike):
Expand Down
38 changes: 38 additions & 0 deletions pandas/tests/groupby/aggregate/test_cython.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,3 +236,41 @@ def test_cython_with_timestamp_and_nat(op, data):

result = df.groupby("a").aggregate(op)
tm.assert_frame_equal(expected, result)


@pytest.mark.parametrize(
"agg",
[
"min",
"max",
"count",
"sum",
"prod",
"var",
"mean",
"median",
"ohlc",
"cumprod",
"cumsum",
"shift",
"any",
"all",
"quantile",
"first",
"last",
"rank",
"cummin",
"cummax",
],
)
def test_read_only_buffer_source_agg(agg):
# https://github.com/pandas-dev/pandas/issues/36014
df = DataFrame(
{
"sepal_length": [5.1, 4.9, 4.7, 4.6, 5.0],
"species": ["setosa", "setosa", "setosa", "setosa", "setosa"],
}
)
df._mgr.blocks[0].values.flags.writeable = False

df.groupby(["species"]).agg({"sepal_length": agg})