gh-114894: add array.array.clear() method by mikeziminio · Pull Request #114919 · python/cpython

mikeziminio · 2024-02-02T15:44:50Z

Issue: Add array.clear() #114894

JelleZijlstra · 2024-02-02T15:51:00Z

Modules/arraymodule.c

    return (PyObject *)np;
 }

+/*[clinic input]


This will also need new unit tests (probably in test_array.py but I haven't checked) and a documentation entry (in array.rst)

This will also need new unit tests (probably in test_array.py but I haven't checked) and a documentation entry (in array.rst)

Thanks for the comment. I'll try to do it now.

mikeziminio · 2024-02-02T19:15:29Z

@JelleZijlstra Jelle, I've added tests and documentation)

JelleZijlstra

Can you also add this to the What's New document for Python 3.13 (Doc/whatsnew/3.13.rst)?

JelleZijlstra · 2024-02-03T04:16:34Z

Lib/test/test_array.py

+        a = array.array(self.typecode, self.example)
+        self.assertRaises(TypeError, a.clear, 42)
+        a.clear()
+        self.assertEqual(a, array.array(self.typecode))


This is a bit of an indirect test, why not instead test that len(a) == 0?

All the testing logic from the test_array.py file works so that the methods are tested for all possible typecode.

To be honest, initially I used array comparison expecting that there would be a typecode check, but then I looked at array_richcompare() and realized that comparing arrays using the == operator is not suitable for checking the same typecode.

Therefore, I added two checks - the len() you suggested and the typecode comparison (just in case of future optimizations).

JelleZijlstra · 2024-02-03T04:17:22Z

Lib/test/test_array.py

+        a.append(self.example[2])
+        a.append(self.example[3])
+        self.assertEqual(a, array.array(self.typecode, self.example[2:4]))
+


Add a test that an array with exported buffers cannot be cleared, something like this (untested):

with memoryview(a): with self.assertRaises(BufferError): a.clear()

Misc/NEWS.d/next/Library/2024-02-02-15-50-13.gh-issue-114894.DF-dSd.rst

Modules/arraymodule.c

aisk · 2024-02-03T12:52:34Z

In my personal view, according to the original issue, the intention behind adding the clear method is to allow array.array to implement collections.abc.MutableSequence, so we can add a test to test it and ensure we don't break it in the future.

There is already a relevant test case in

cpython/Lib/test/test_collections.py

Line 1968 in 28bb296

def test_MutableSequence(self):

, so what we need to do is simply include array.array in this existing test case.

Co-authored-by: AN Long <aisk@users.noreply.github.com>

…F-dSd.rst Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

mikeziminio · 2024-02-06T13:27:11Z

In my personal view, according to the original issue, the intention behind adding the clear method is to allow array.array to implement collections.abc.MutableSequence, so we can add a test to test it and ensure we don't break it in the future.

There is already a relevant test case in

cpython/Lib/test/test_collections.py

Line 1968 in 28bb296

def test_MutableSequence(self):

, so what we need to do is simply include array.array in this existing test case.

done

I added only the issubclass check (without isinstance), because for isinstance it would have to be check all typecodes.

aisk · 2024-02-06T13:56:01Z

Lib/test/test_collections.py

 from random import choice, randrange
 from itertools import product, chain, combinations
 import string
+import array


Nitpick: These imports are basically sorted alphabetically, so this line may move to the first line.

Doc/whatsnew/3.13.rst

Doc/library/array.rst

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

mikeziminio · 2024-02-08T13:19:41Z

@JelleZijlstra @aisk Guys, if I understand correctly, I made all the changes according to the reviews.
Please tell me what further steps are needed to merge the PR.

aisk · 2024-02-08T13:30:47Z

LGTM, I think the next step is to wait for a core member to approve and merge it.

Jason-Y-Z · 2024-02-10T15:40:49Z

Thanks all for the efforts, sorry for tagging but gentle nudge to @vstinner who seems to have touched the relevant files recently (according to BLAME).

Jason-Y-Z · 2024-02-10T16:05:50Z

Thanks!

Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: AN Long <aisk@users.noreply.github.com> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

pythongh-114894: add array.array.clear() method

be9733a

bedevere-app bot added the awaiting review label Feb 2, 2024

bedevere-app bot mentioned this pull request Feb 2, 2024

Add array.clear() #114894

Closed

📜🤖 Added by blurb_it.

9c2c360

JelleZijlstra reviewed Feb 2, 2024

View reviewed changes

mikeziminio force-pushed the fix-issue-114894 branch from 3368a6b to 9c2c360 Compare February 2, 2024 18:51

mikeziminio added 2 commits February 2, 2024 22:54

added test and documentation

8adac42

fixed - trim trailing whitespace

e04c2fa

JelleZijlstra self-requested a review February 2, 2024 19:28

JelleZijlstra reviewed Feb 3, 2024

View reviewed changes

aisk reviewed Feb 3, 2024

View reviewed changes

Modules/arraymodule.c Outdated Show resolved Hide resolved

mikeziminio and others added 4 commits February 6, 2024 09:32

Update Modules/arraymodule.c

7630069

Co-authored-by: AN Long <aisk@users.noreply.github.com>

Update Misc/NEWS.d/next/Library/2024-02-02-15-50-13.gh-issue-114894.D…

556bf1a

…F-dSd.rst Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

add memoryview test, update other tests

b5b9e63

add MutableSequence test

83422b0

mikeziminio requested a review from rhettinger as a code owner February 6, 2024 12:11

mikeziminio added 2 commits February 6, 2024 16:14

resolve clinic conflicts

4d5fcfb

add whatsnew doc

519ad21

mikeziminio requested review from JelleZijlstra and aisk February 6, 2024 13:28

change issue reference

6242242

aisk reviewed Feb 6, 2024

View reviewed changes

move so that it matches the alphabetical order.

fc76d73

rhettinger removed their request for review February 6, 2024 16:16

JelleZijlstra reviewed Feb 7, 2024

View reviewed changes

Doc/whatsnew/3.13.rst Outdated Show resolved Hide resolved

Doc/library/array.rst Show resolved Hide resolved

mikeziminio and others added 2 commits February 7, 2024 07:19

Update Doc/whatsnew/3.13.rst

7b46716

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

Update Doc/library/array.rst

a399ac6

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

JelleZijlstra approved these changes Feb 10, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting review labels Feb 10, 2024

JelleZijlstra merged commit 9d1a353 into python:main Feb 10, 2024

bedevere-app bot removed the awaiting merge label Feb 10, 2024

Uh oh!

Conversation

mikeziminio commented Feb 2, 2024 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikeziminio commented Feb 2, 2024

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aisk commented Feb 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikeziminio commented Feb 6, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mikeziminio commented Feb 8, 2024

Uh oh!

aisk commented Feb 8, 2024

Uh oh!

Jason-Y-Z commented Feb 10, 2024

Uh oh!

Jason-Y-Z commented Feb 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mikeziminio commented Feb 2, 2024 •

edited by bedevere-app bot

Loading

aisk commented Feb 3, 2024 •

edited

Loading