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

[RFC] Render typed iterators in docstrings (alternative) #2371

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft
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
78 changes: 69 additions & 9 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -1712,6 +1712,22 @@ struct iterator_state {
bool first_or_done;
};

template<typename Iterator, typename Sentinel, return_value_policy Policy>
struct type_caster<iterator_state<Iterator, Sentinel, false, Policy>> :
type_caster_base<iterator_state<Iterator, Sentinel, false, Policy>> {
using ValueType = decltype(*std::declval<Iterator>());
public:
static constexpr auto name = _("Iterator[") + make_caster<ValueType>::name + _("]");
};

template<typename Iterator, typename Sentinel, return_value_policy Policy>
struct type_caster<iterator_state<Iterator, Sentinel, true, Policy>> :
type_caster_base<iterator_state<Iterator, Sentinel, true, Policy>> {
using ValueType = decltype((*std::declval<Iterator>()).first);
public:
static constexpr auto name = _("Iterator[") + make_caster<ValueType>::name + _("]");
};

PYBIND11_NAMESPACE_END(detail)

/// Makes a python iterator from a first and past-the-end C++ InputIterator.
Expand All @@ -1720,7 +1736,8 @@ template <return_value_policy Policy = return_value_policy::reference_internal,
typename Sentinel,
typename ValueType = decltype(*std::declval<Iterator>()),
typename... Extra>
iterator make_iterator(Iterator first, Sentinel last, Extra &&... extra) {
detail::iterator_state<Iterator, Sentinel, false, Policy>
make_iterator_ng(Iterator first, Sentinel last, Extra &&... extra) {
typedef detail::iterator_state<Iterator, Sentinel, false, Policy> state;

if (!detail::get_type_info(typeid(state), false)) {
Expand All @@ -1738,8 +1755,7 @@ iterator make_iterator(Iterator first, Sentinel last, Extra &&... extra) {
return *s.it;
}, std::forward<Extra>(extra)..., Policy);
}

return cast(state{first, last, true});
return state{first, last, true};
}

/// Makes an python iterator over the keys (`.first`) of a iterator over pairs from a
Expand All @@ -1749,7 +1765,8 @@ template <return_value_policy Policy = return_value_policy::reference_internal,
typename Sentinel,
typename KeyType = decltype((*std::declval<Iterator>()).first),
typename... Extra>
iterator make_key_iterator(Iterator first, Sentinel last, Extra &&... extra) {
detail::iterator_state<Iterator, Sentinel, true, Policy>
make_key_iterator_ng(Iterator first, Sentinel last, Extra &&... extra) {
typedef detail::iterator_state<Iterator, Sentinel, true, Policy> state;

if (!detail::get_type_info(typeid(state), false)) {
Expand All @@ -1768,23 +1785,66 @@ iterator make_key_iterator(Iterator first, Sentinel last, Extra &&... extra) {
}, std::forward<Extra>(extra)..., Policy);
}

return cast(state{first, last, true});
return state{first, last, true};
}

/// Makes a python iterator from a first and past-the-end C++ InputIterator.
template <return_value_policy Policy = return_value_policy::reference_internal,
typename Iterator,
typename Sentinel,
typename ValueType = decltype(*std::declval<Iterator>()),
typename... Extra>
PYBIND11_DEPRECATED("Superseded by make_iterator_ng")
iterator make_iterator(Iterator first, Sentinel last, Extra &&... extra) {
return cast(make_iterator_ng(first, last, std::forward<Extra>(extra)...));
}

/// Makes a python iterator from a first and past-the-end C++ InputIterator.
template <return_value_policy Policy = return_value_policy::reference_internal,
typename Iterator,
typename Sentinel,
typename ValueType = decltype(*std::declval<Iterator>()),
typename... Extra>
PYBIND11_DEPRECATED("Superseded by make_key_iterator_ng")
iterator make_key_iterator(Iterator first, Sentinel last, Extra &&... extra) {
return cast(make_key_iterator_ng(first, last, std::forward<Extra>(extra)...));
}

template <return_value_policy Policy = return_value_policy::reference_internal,
typename Type, typename... Extra>
detail::iterator_state<decltype(std::begin(std::declval<Type &>())), decltype(std::end(std::declval<Type &>())), false, Policy>
make_iterator_ng(Type &value, Extra &&... extra) {
return make_iterator_ng<Policy>(std::begin(value), std::end(value), std::forward<Extra>(extra)...);
}

/// Makes an iterator over the keys (`.first`) of a stl map-like container supporting
/// `std::begin()`/`std::end()`
template<return_value_policy Policy = return_value_policy::reference_internal,
typename Type, typename... Extra>
detail::iterator_state<decltype(std::begin(std::declval<Type&>())), decltype(std::end(std::declval<Type&>())), true, Policy>
make_key_iterator_ng(Type &value, Extra &&... extra) {
return make_key_iterator_ng<Policy>(std::begin(value), std::end(value), std::forward<Extra>(extra)...);
}

/// Makes an iterator over values of an stl container or other container supporting
/// `std::begin()`/`std::end()`
template <return_value_policy Policy = return_value_policy::reference_internal,
typename Type, typename... Extra> iterator make_iterator(Type &value, Extra&&... extra) {
return make_iterator<Policy>(std::begin(value), std::end(value), extra...);
typename Type, typename... Extra>
PYBIND11_DEPRECATED("Superseded by make_iterator_ng")
iterator make_iterator(Type &value, Extra&&... extra) {
return cast(make_iterator_ng<Policy>(value, std::forward<Extra>(extra)...));
}

/// Makes an iterator over the keys (`.first`) of a stl map-like container supporting
/// `std::begin()`/`std::end()`
template <return_value_policy Policy = return_value_policy::reference_internal,
typename Type, typename... Extra> iterator make_key_iterator(Type &value, Extra&&... extra) {
return make_key_iterator<Policy>(std::begin(value), std::end(value), extra...);
typename Type, typename... Extra>
PYBIND11_DEPRECATED("Superseded by make_key_iterator_ng")
iterator make_key_iterator(Type &value, Extra&&... extra) {
return cast(make_key_iterator<Policy>(value, std::forward<Extra>(extra)...));
}


template <typename InputType, typename OutputType> void implicitly_convertible() {
struct set_flag {
bool &flag;
Expand Down
8 changes: 4 additions & 4 deletions include/pybind11/stl_bind.h
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ void vector_accessor(enable_if_t<!vector_needs_copy<Vector>::value, Class_> &cl)

cl.def("__iter__",
[](Vector &v) {
return make_iterator<
return make_iterator_ng<
return_value_policy::reference_internal, ItType, ItType, T&>(
v.begin(), v.end());
},
Expand All @@ -340,7 +340,7 @@ void vector_accessor(enable_if_t<vector_needs_copy<Vector>::value, Class_> &cl)

cl.def("__iter__",
[](Vector &v) {
return make_iterator<
return make_iterator_ng<
return_value_policy::copy, ItType, ItType, T>(
v.begin(), v.end());
},
Expand Down Expand Up @@ -608,12 +608,12 @@ class_<Map, holder_type> bind_map(handle scope, const std::string &name, Args&&.
);

cl.def("__iter__",
[](Map &m) { return make_key_iterator(m.begin(), m.end()); },
[](Map &m) { return make_key_iterator_ng(m.begin(), m.end()); },
keep_alive<0, 1>() /* Essential: keep list alive while iterator exists */
);

cl.def("items",
[](Map &m) { return make_iterator(m.begin(), m.end()); },
[](Map &m) { return make_iterator_ng(m.begin(), m.end()); },
keep_alive<0, 1>() /* Essential: keep list alive while iterator exists */
);

Expand Down
2 changes: 1 addition & 1 deletion tests/test_opaque_types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ TEST_SUBMODULE(opaque_types, m) {
.def("back", (std::string &(StringList::*)()) &StringList::back)
.def("__len__", [](const StringList &v) { return v.size(); })
.def("__iter__", [](StringList &v) {
return py::make_iterator(v.begin(), v.end());
return py::make_iterator_ng(v.begin(), v.end());
}, py::keep_alive<0, 1>());

class ClassWithSTLVecProperty {
Expand Down
16 changes: 8 additions & 8 deletions tests/test_sequences_and_iterators.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ TEST_SUBMODULE(sequences_and_iterators, m) {
})
.def("__len__", &Sequence::size)
/// Optional sequence protocol operations
.def("__iter__", [](const Sequence &s) { return py::make_iterator(s.begin(), s.end()); },
.def("__iter__", [](const Sequence &s) { return py::make_iterator_ng(s.begin(), s.end()); },
py::keep_alive<0, 1>() /* Essential: keep object alive while iterator exists */)
.def("__contains__", [](const Sequence &s, float v) { return s.contains(v); })
.def("__reversed__", [](const Sequence &s) -> Sequence { return s.reversed(); })
Expand Down Expand Up @@ -249,9 +249,9 @@ TEST_SUBMODULE(sequences_and_iterators, m) {
})
.def("__setitem__", &StringMap::set)
.def("__len__", &StringMap::size)
.def("__iter__", [](const StringMap &map) { return py::make_key_iterator(map.begin(), map.end()); },
.def("__iter__", [](const StringMap &map) { return py::make_key_iterator_ng(map.begin(), map.end()); },
py::keep_alive<0, 1>())
.def("items", [](const StringMap &map) { return py::make_iterator(map.begin(), map.end()); },
.def("items", [](const StringMap &map) { return py::make_iterator_ng(map.begin(), map.end()); },
py::keep_alive<0, 1>())
;

Expand All @@ -266,10 +266,10 @@ TEST_SUBMODULE(sequences_and_iterators, m) {
py::class_<IntPairs>(m, "IntPairs")
.def(py::init<std::vector<std::pair<int, int>>>())
.def("nonzero", [](const IntPairs& s) {
return py::make_iterator(NonZeroIterator<std::pair<int, int>>(s.begin()), NonZeroSentinel());
return py::make_iterator_ng(NonZeroIterator<std::pair<int, int>>(s.begin()), NonZeroSentinel());
}, py::keep_alive<0, 1>())
.def("nonzero_keys", [](const IntPairs& s) {
return py::make_key_iterator(NonZeroIterator<std::pair<int, int>>(s.begin()), NonZeroSentinel());
return py::make_key_iterator_ng(NonZeroIterator<std::pair<int, int>>(s.begin()), NonZeroSentinel());
}, py::keep_alive<0, 1>())
;

Expand Down Expand Up @@ -345,12 +345,12 @@ TEST_SUBMODULE(sequences_and_iterators, m) {
// test_iterator_passthrough
// #181: iterator passthrough did not compile
m.def("iterator_passthrough", [](py::iterator s) -> py::iterator {
return py::make_iterator(std::begin(s), std::end(s));
return py::cast(py::make_iterator_ng(std::begin(s), std::end(s)));
});

// test_iterator_rvp
// #388: Can't make iterators via make_iterator() with different r/v policies
static std::vector<int> list = { 1, 2, 3 };
m.def("make_iterator_1", []() { return py::make_iterator<py::return_value_policy::copy>(list); });
m.def("make_iterator_2", []() { return py::make_iterator<py::return_value_policy::automatic>(list); });
m.def("make_iterator_1", []() { return py::make_iterator_ng<py::return_value_policy::copy>(list); });
m.def("make_iterator_2", []() { return py::make_iterator_ng<py::return_value_policy::automatic>(list); });
}
29 changes: 29 additions & 0 deletions tests/test_stl_binders.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,3 +277,32 @@ def test_map_delitem():
del um['ua']
assert sorted(list(um)) == ['ub']
assert sorted(list(um.items())) == [('ub', 2.6)]


def test_map_docstrings(doc):
assert (doc(m.MapStringDouble.__iter__) ==
"__iter__(self: m.stl_binders.MapStringDouble)"
" -> Iterator[str]")
assert (doc(m.MapStringDouble.items) ==
"items(self: m.stl_binders.MapStringDouble)"
" -> Iterator[Tuple[str, float]]")
assert (doc(m.UnorderedMapStringDouble.__iter__) ==
"__iter__(self: m.stl_binders.UnorderedMapStringDouble)"
" -> Iterator[str]\n")
assert (doc(m.UnorderedMapStringDouble.items) ==
"items(self: m.stl_binders.UnorderedMapStringDouble)"
" -> Iterator[Tuple[str, float]]\n")


def test_vector_docstrings(doc):
assert (doc(m.VectorInt.__iter__) ==
"__iter__(self: m.stl_binders.VectorInt)"
" -> Iterator[int]\n")


@pytest.unsupported_on_pypy
@pytest.requires_numpy
def test_vector_docstring2(doc):
assert (doc(m.VectorStruct.__iter__) ==
"__iter__(self: m.stl_binders.VectorStruct)"
" -> Iterator[m.stl_binders.VStruct]")