-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add list.clear() in the C++ API. #30120
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Adding |
It seems now that some integration tests (Windows, MacOS) fail for unrelated reasons... how can I help fixing? |
Sorry. I've been out sick for the last few days. This PR is already on my todo list, though. Monday is a public holiday for me. I'll try to get to this when I'm back at work on Tuesday. |
I had a quick look at the tests but can't immediately figure out what's wrong. I don't think fixing those is highest priority right now. So I think we just ignore them. However, I'd like to wait merging this PR. I would like to catch up with Ralf first. In general we'd like this repo to have a few differences from https://github.com/pybind/pybind11 as possible. Ideally we can contribute all changes back upstream and delete this fork eventually. This PR would be one more thing for us to upstream. This might be fine, but I think I need to understand the consequences a little better before. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please transfer this PR to a https://github.com/pybind/pybind11 PR (upstream pybind11)?
It's still very important that we keep the delta between upstream pybind11 master and pybind11clif as small as possible.
@@ -2195,6 +2195,7 @@ class list : public object { | |||
throw error_already_set(); | |||
} | |||
} | |||
void clear() { PyList_SetSlice(m_ptr, 0, PyList_Size(m_ptr), nullptr); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return value of PyList_SetSlice()
needs to be checked, and you need to add throw error_already_set();
if it is not 0
:
https://docs.python.org/3/c-api/list.html#c.PyList_SetSlice
(I think it's fine/best to omit a similar check for PyList_Size()
: from the context we're sure that m_ptr
is a Python list
object.)
This change was merged as pybind/pybind11#5153. I'll merge upstream master here soon. Closing this PR. |
Description
Add list.clear() in the C++ API, this allows resetting a list (e.g. for doing function argument modification and such).
Append() is already present but there is currently no way to alter or remove items from a list.
Suggested changelog entry:
Add pybind11::list::clear() in C++.