Accept Python iterables and sequences as C++ sets and vectors #30042
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rwgk
reviewed
May 23, 2023
| void reserve_maybe(const sequence &, void *) {} | ||
|
|
||
| template <typename ContainerType> | ||
| bool insert_elements(const ContainerType &container, bool convert) { |
Contributor
There was a problem hiding this comment.
Could we move this to the private: section?
Contributor
Author
There was a problem hiding this comment.
Looks like it is already in the private section. I moved the other insert_elements to private.
Contributor
There was a problem hiding this comment.
oh, sorry, it's so difficult to see in the GH view
include/pybind11/stl.h
Outdated
| } | ||
| return true; | ||
| } | ||
| if (isinstance<sequence>(src)) { |
Contributor
There was a problem hiding this comment.
Below we're checking isinstance<sequence> first, which seems better.
In any case, I'd consistently use the same order, just to have one potential surprise less to worry about.
include/pybind11/stl.h
Outdated
| reserve_maybe(s, &value); | ||
| for (auto it : s) { | ||
| template <typename ContainerType> | ||
| bool insert_elements(const ContainerType &container, bool convert) { |
rwgk
approved these changes
May 23, 2023
rwgk
added a commit
to rwgk/pybind11
that referenced
this pull request
May 30, 2023
…rc.ptr()`). Note for completeness: This is a more conservative change than google/pybind11clif#30042
rwgk
added a commit
to rwgk/pybind11
that referenced
this pull request
Aug 31, 2023
…rc.ptr()`). Note for completeness: This is a more conservative change than google/pybind11clif#30042
rwgk
added a commit
to pybind/pybind11
that referenced
this pull request
Aug 13, 2024
* Add `test_pass_std_vector_int()`, `test_pass_std_set_int()` in test_stl * Change `list_caster` to also accept generator objects (`PyGen_Check(src.ptr()`). Note for completeness: This is a more conservative change than google/pybind11clif#30042 * Drop in (currently unpublished) PyCLIF code, use in `list_caster`, adjust tests. * Use `PyObjectTypeIsConvertibleToStdSet()` in `set_caster`, adjust tests. * Use `PyObjectTypeIsConvertibleToStdMap()` in `map_caster`, add tests. * Simplify `list_caster` `load()` implementation, push str/bytes check into `PyObjectTypeIsConvertibleToStdVector()`. * clang-tidy cleanup with a few extra `(... != 0)` to be more consistent. * Also use `PyObjectTypeIsConvertibleToStdVector()` in `array_caster`. * Update comment pointing to clif/python/runtime.cc (code is unchanged). * Comprehensive test coverage, enhanced set_caster load implementation. * Resolve clang-tidy eror. * Add a long C++ comment explaining what led to the `PyObjectTypeIsConvertibleTo*()` implementations. * Minor function name change in test. * strcmp -> std::strcmp (thanks @Skylion007 for catching this) * Add `PyCallable_Check(items)` in `PyObjectTypeIsConvertibleToStdMap()` * Resolve clang-tidy error * Use `PyMapping_Items()` instead of `src.attr("items")()`, to be internally consistent with `PyMapping_Check()` * Update link to PyCLIF sources. * Fix typo (thanks @wangxf123456 for catching this) * Add `test_pass_std_vector_int()`, `test_pass_std_set_int()` in test_stl * Change `list_caster` to also accept generator objects (`PyGen_Check(src.ptr()`). Note for completeness: This is a more conservative change than google/pybind11clif#30042 * Drop in (currently unpublished) PyCLIF code, use in `list_caster`, adjust tests. * Use `PyObjectTypeIsConvertibleToStdSet()` in `set_caster`, adjust tests. * Use `PyObjectTypeIsConvertibleToStdMap()` in `map_caster`, add tests. * Simplify `list_caster` `load()` implementation, push str/bytes check into `PyObjectTypeIsConvertibleToStdVector()`. * clang-tidy cleanup with a few extra `(... != 0)` to be more consistent. * Also use `PyObjectTypeIsConvertibleToStdVector()` in `array_caster`. * Update comment pointing to clif/python/runtime.cc (code is unchanged). * Comprehensive test coverage, enhanced set_caster load implementation. * Resolve clang-tidy eror. * Add a long C++ comment explaining what led to the `PyObjectTypeIsConvertibleTo*()` implementations. * Minor function name change in test. * strcmp -> std::strcmp (thanks @Skylion007 for catching this) * Add `PyCallable_Check(items)` in `PyObjectTypeIsConvertibleToStdMap()` * Resolve clang-tidy error * Use `PyMapping_Items()` instead of `src.attr("items")()`, to be internally consistent with `PyMapping_Check()` * Update link to PyCLIF sources. * Fix typo (thanks @wangxf123456 for catching this) * Fix typo discovered by new version of codespell.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Note that we refuse Python dictionaries as C++ sets or vectors. This is to be consistent with PyCLIF: https://github.com/google/clif/blob/0521e3c150c1e7fff82a8df8929660c58eec544b/clif/python/stltypes.h#L862.