Skip to content
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.

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

gh-125420: implement Sequence.count API on memoryview objects #125443

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions Doc/library/stdtypes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4140,6 +4140,12 @@ copying.
.. versionchanged:: 3.5
The source format is no longer restricted when casting to a byte view.

.. method:: count(value, /)

Count the number of occurrences of *value*.

.. versionadded:: next

There are also several readonly attributes available:

.. attribute:: obj
Expand Down
28 changes: 28 additions & 0 deletions Lib/test/test_memoryview.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,22 @@ def test_iter(self):
m = self._view(b)
self.assertEqual(list(m), [m[i] for i in range(len(m))])

def test_count(self):
for tp in self._types:
b = tp(self._source)
m = self._view(b)
l = m.tolist()
for ch in list(m):
self.assertEqual(m.count(ch), l.count(ch))

b = tp((b'a' * 5) + (b'c' * 3))
m = self._view(b) # may be sliced
l = m.tolist()
with self.subTest('count', buffer=b):
self.assertEqual(m.count(ord('a')), l.count(ord('a')))
picnixz marked this conversation as resolved.
Show resolved Hide resolved
self.assertEqual(m.count(ord('b')), l.count(ord('b')))
self.assertEqual(m.count(ord('c')), l.count(ord('c')))

def test_setitem_readonly(self):
if not self.ro_type:
self.skipTest("no read-only type to test")
Expand Down Expand Up @@ -438,6 +454,18 @@ def _view(self, obj):
def _check_contents(self, tp, obj, contents):
self.assertEqual(obj, tp(contents))

def test_count(self):
super().test_count()
for tp in self._types:
b = tp((b'a' * 5) + (b'c' * 3))
m = self._view(b) # should not be sliced
self.assertEqual(len(b), len(m))
with self.subTest('count', buffer=b):
self.assertEqual(m.count(ord('a')), 5)
self.assertEqual(m.count(ord('b')), 0)
self.assertEqual(m.count(ord('c')), 3)


class BaseMemorySliceTests:
source_bytes = b"XabcdefY"

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Add :meth:`memoryview.count` to :class:`memoryview` objects. Patch by
Bénédikt Tran.
11 changes: 10 additions & 1 deletion Objects/clinic/memoryobject.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 50 additions & 0 deletions Objects/memoryobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -2748,6 +2748,55 @@ static PySequenceMethods memory_as_sequence = {
};


/****************************************************************************/
/* Counting */
/****************************************************************************/

/*[clinic input]
memoryview.count
corona10 marked this conversation as resolved.
Show resolved Hide resolved

value: object
/

Count the number of occurrences of a value.
[clinic start generated code]*/

static PyObject *
memoryview_count(PyMemoryViewObject *self, PyObject *value)
/*[clinic end generated code: output=e2c255a8d54eaa12 input=e3036ce1ed7d1823]*/
{
PyObject *iter = PyObject_GetIter(_PyObject_CAST(self));
if (iter == NULL) {
return NULL;
}

Py_ssize_t count = 0;
PyObject *item = NULL;
while (PyIter_NextItem(iter, &item)) {
Copy link
Member

Choose a reason for hiding this comment

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

We could probably make this significantly more efficient by looking at the memory directly without materializing an iterator and all the elements. str.count is a similar example. However, that is going to be a lot more complicated, so it seems fine to skip it; if somebody wants to optimize this method later they can.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's what I suspected so I went for the simplest approach (at least to have the code around).

if (item == NULL) {
Py_DECREF(iter);
return NULL;
}
if (item == value) {
Py_DECREF(item);
count++; // no overflow since count <= len(mv) <= PY_SSIZE_T_MAX
continue;
}
int contained = PyObject_RichCompareBool(item, value, Py_EQ);
Copy link
Member

Choose a reason for hiding this comment

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

list.count has a special case for if (item == value) count++. I suspect that helps significantly here too, because the value item will usually be an interned integer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, PyObject_RichCompareBool has that fast path inside as well. I had the fast path before but then I wondered whether it was really needed or not (and I was a bit too lazy for benchmarkings). But I forgot about the interned integer case so it's probably faster.

Py_DECREF(item);
if (contained > 0) { // more likely than 'contained < 0'
count++; // no overflow since count <= len(mv) <= PY_SSIZE_T_MAX
}
else if (contained < 0) {
Py_DECREF(iter);
return NULL;
}
}
Py_DECREF(iter);
return PyLong_FromSsize_t(count);
}


/**************************************************************************/
/* Comparisons */
/**************************************************************************/
Expand Down Expand Up @@ -3284,6 +3333,7 @@ static PyMethodDef memory_methods[] = {
MEMORYVIEW_CAST_METHODDEF
MEMORYVIEW_TOREADONLY_METHODDEF
MEMORYVIEW__FROM_FLAGS_METHODDEF
MEMORYVIEW_COUNT_METHODDEF
{"__enter__", memory_enter, METH_NOARGS, NULL},
{"__exit__", memory_exit, METH_VARARGS, memory_exit_doc},
{NULL, NULL}
Expand Down
Loading