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

feat: add a pybind function to clear a list. #5153

Merged
merged 4 commits into from
Jun 7, 2024
Merged

Conversation

TCKnet
Copy link
Contributor

@TCKnet TCKnet commented Jun 3, 2024

Description

Add py::list::clear().

See also: https://stackoverflow.com/questions/23489177/how-to-clear-a-pylistobject

Suggested changelog entry:

``py::list`` gained a ``.clear()`` method.

@TCKnet
Copy link
Contributor Author

TCKnet commented Jun 6, 2024

It seems that this is a system/test configuration/version issue rather than an issue with the CL per se. Could a maintainer have a look? Tthat would be awesome! Thank you very much in advance!

Using cmake version 3.29.4
Error: Could not find linux asset for cmake version 3.29.4
Using cmake version 3.29.4
Error: Could not find darwin asset for cmake version 3.29.4

@rwgk
Copy link
Collaborator

rwgk commented Jun 6, 2024

About to close/reopen this PR to trigger a fresh rerun of the GitHub Actions.

@rwgk rwgk closed this Jun 6, 2024
@rwgk rwgk reopened this Jun 6, 2024
Copy link
Collaborator

@rwgk rwgk left a comment

Choose a reason for hiding this comment

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

LGTM. I think this is a useful small addition, especially because the implementation isn't exactly obvious (why is there no PyList_Clear()?).

@@ -2183,6 +2183,11 @@ class list : public object {
throw error_already_set();
}
}
void clear() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add the non-const comment the other pybind11 clear methods have?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done. Thanks for catching this!

Copy link
Collaborator

@Skylion007 Skylion007 left a comment

Choose a reason for hiding this comment

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

Except for the comment issue, looks good.

@rwgk rwgk merged commit 35ff42b into pybind:master Jun 7, 2024
83 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Jun 7, 2024
@henryiii henryiii changed the title Add a pybind function to clear a list. feat: add a pybind function to clear a list. Jun 23, 2024
@henryiii henryiii removed the needs changelog Possibly needs a changelog entry label Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants