From 0fb981b21966339b811bc770b11f71eddd918f13 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Wed, 22 Sep 2021 15:38:50 -0400 Subject: [PATCH 1/5] Add blacken-docs and pycln pre-commit hooks (#3292) * Apply blacken-docs and fix language-hints * Add blacken-docs pre-commit hook * Add pycln pre-commit hook * Enable a few builtin hooks * Black no longer ignores pyi files --- .pre-commit-config.yaml | 30 ++++++++++++++++++++--------- docs/advanced/cast/custom.rst | 2 ++ docs/advanced/cast/eigen.rst | 4 ++-- docs/advanced/cast/strings.rst | 32 +++++++++++++++---------------- docs/advanced/classes.rst | 8 +++++--- docs/advanced/embedding.rst | 1 + docs/advanced/functions.rst | 7 ++++--- docs/advanced/pycpp/numpy.rst | 6 +++--- docs/advanced/pycpp/object.rst | 1 + docs/advanced/pycpp/utilities.rst | 2 +- docs/advanced/smart_ptrs.rst | 1 + docs/benchmark.py | 1 - docs/changelog.rst | 1 + docs/classes.rst | 22 ++++++++++----------- docs/compiling.rst | 11 ++--------- docs/conf.py | 1 - docs/faq.rst | 2 +- 17 files changed, 72 insertions(+), 60 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b9e37e5fe..4245641ba 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -19,8 +19,10 @@ repos: hooks: - id: check-added-large-files - id: check-case-conflict + - id: check-docstring-first - id: check-merge-conflict - id: check-symlinks + - id: check-toml - id: check-yaml - id: debug-statements - id: end-of-file-fixer @@ -42,12 +44,16 @@ repos: # Black, the code formatter, natively supports pre-commit - repo: https://github.com/psf/black - rev: 21.9b0 + rev: 21.9b0 # Keep in sync with blacken-docs hooks: - id: black - # By default, this ignores pyi files, though black supports them - types: [text] - files: \.pyi?$ + +- repo: https://github.com/asottile/blacken-docs + rev: v1.11.0 + hooks: + - id: blacken-docs + additional_dependencies: + - black==21.9b0 # keep in sync with black hook # Changes tabs to spaces - repo: https://github.com/Lucas-C/pre-commit-hooks @@ -55,6 +61,12 @@ repos: hooks: - id: remove-tabs +# Autoremoves unused imports +- repo: https://github.com/hadialqattan/pycln + rev: v1.0.3 + hooks: + - id: pycln + # Flake8 also supports pre-commit natively (same author) - repo: https://github.com/PyCQA/flake8 rev: 3.9.2 @@ -86,7 +98,7 @@ repos: # Checks the manifest for missing files (native support) - repo: https://github.com/mgedmin/check-manifest - rev: "0.46" + rev: "0.47" hooks: - id: check-manifest # This is a slow hook, so only run this if --hook-stage manual is passed @@ -100,10 +112,10 @@ repos: exclude: ".supp$" args: ["-L", "nd,ot,thist"] -- repo: https://github.com/shellcheck-py/shellcheck-py - rev: v0.7.2.1 - hooks: - - id: shellcheck +- repo: https://github.com/shellcheck-py/shellcheck-py + rev: v0.7.2.1 + hooks: + - id: shellcheck # The original pybind11 checks for a few C++ style items - repo: local diff --git a/docs/advanced/cast/custom.rst b/docs/advanced/cast/custom.rst index a779444c2..19b935347 100644 --- a/docs/advanced/cast/custom.rst +++ b/docs/advanced/cast/custom.rst @@ -26,7 +26,9 @@ The following Python snippet demonstrates the intended usage from the Python sid def __int__(self): return 123 + from example import print + print(A()) To register the necessary conversion routines, it is necessary to add an diff --git a/docs/advanced/cast/eigen.rst b/docs/advanced/cast/eigen.rst index e01472d5a..80f101343 100644 --- a/docs/advanced/cast/eigen.rst +++ b/docs/advanced/cast/eigen.rst @@ -112,7 +112,7 @@ example: .. code-block:: python a = MyClass() - m = a.get_matrix() # flags.writeable = True, flags.owndata = False + m = a.get_matrix() # flags.writeable = True, flags.owndata = False v = a.view_matrix() # flags.writeable = False, flags.owndata = False c = a.copy_matrix() # flags.writeable = True, flags.owndata = True # m[5,6] and v[5,6] refer to the same element, c[5,6] does not. @@ -203,7 +203,7 @@ adding the ``order='F'`` option when creating an array: .. code-block:: python - myarray = np.array(source, order='F') + myarray = np.array(source, order="F") Such an object will be passable to a bound function accepting an ``Eigen::Ref`` (or similar column-major Eigen type). diff --git a/docs/advanced/cast/strings.rst b/docs/advanced/cast/strings.rst index e25701eca..cfd7e7b7a 100644 --- a/docs/advanced/cast/strings.rst +++ b/docs/advanced/cast/strings.rst @@ -36,13 +36,13 @@ everywhere `_. } ); -.. code-block:: python +.. code-block:: pycon - >>> utf8_test('🎂') + >>> utf8_test("🎂") utf-8 is icing on the cake. 🎂 - >>> utf8_charptr('🍕') + >>> utf8_charptr("🍕") My favorite food is 🍕 @@ -80,7 +80,7 @@ raise a ``UnicodeDecodeError``. } ); -.. code-block:: python +.. code-block:: pycon >>> isinstance(example.std_string_return(), str) True @@ -114,7 +114,7 @@ conversion has the same overhead as implicit conversion. } ); -.. code-block:: python +.. code-block:: pycon >>> str_output() 'Send your résumé to Alice in HR' @@ -143,7 +143,7 @@ returned to Python as ``bytes``, then one can return the data as a } ); -.. code-block:: python +.. code-block:: pycon >>> example.return_bytes() b'\xba\xd0\xba\xd0' @@ -160,7 +160,7 @@ encoding, but cannot convert ``std::string`` back to ``bytes`` implicitly. } ); -.. code-block:: python +.. code-block:: pycon >>> isinstance(example.asymmetry(b"have some bytes"), str) True @@ -229,16 +229,16 @@ character. m.def("pass_char", [](char c) { return c; }); m.def("pass_wchar", [](wchar_t w) { return w; }); -.. code-block:: python +.. code-block:: pycon - >>> example.pass_char('A') + >>> example.pass_char("A") 'A' While C++ will cast integers to character types (``char c = 0x65;``), pybind11 does not convert Python integers to characters implicitly. The Python function ``chr()`` can be used to convert integers to characters. -.. code-block:: python +.. code-block:: pycon >>> example.pass_char(0x65) TypeError @@ -259,17 +259,17 @@ a combining acute accent). The combining character will be lost if the two-character sequence is passed as an argument, even though it renders as a single grapheme. -.. code-block:: python +.. code-block:: pycon - >>> example.pass_wchar('é') + >>> example.pass_wchar("é") 'é' - >>> combining_e_acute = 'e' + '\u0301' + >>> combining_e_acute = "e" + "\u0301" >>> combining_e_acute 'é' - >>> combining_e_acute == 'é' + >>> combining_e_acute == "é" False >>> example.pass_wchar(combining_e_acute) @@ -278,9 +278,9 @@ single grapheme. Normalizing combining characters before passing the character literal to C++ may resolve *some* of these issues: -.. code-block:: python +.. code-block:: pycon - >>> example.pass_wchar(unicodedata.normalize('NFC', combining_e_acute)) + >>> example.pass_wchar(unicodedata.normalize("NFC", combining_e_acute)) 'é' In some languages (Thai for example), there are `graphemes that cannot be diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst index 9d1b1f0d9..7f8fcdf4b 100644 --- a/docs/advanced/classes.rst +++ b/docs/advanced/classes.rst @@ -136,7 +136,7 @@ a virtual method call. u'woof! woof! woof! ' >>> class Cat(Animal): ... def go(self, n_times): - ... return "meow! " * n_times + ... return "meow! " * n_times ... >>> c = Cat() >>> call_go(c) @@ -159,8 +159,9 @@ Here is an example: class Dachshund(Dog): def __init__(self, name): - Dog.__init__(self) # Without this, a TypeError is raised. + Dog.__init__(self) # Without this, a TypeError is raised. self.name = name + def bark(self): return "yap!" @@ -1153,6 +1154,7 @@ error: >>> class PyFinalChild(IsFinal): ... pass + ... TypeError: type 'IsFinal' is not an acceptable base type .. note:: This attribute is currently ignored on PyPy @@ -1247,7 +1249,7 @@ Accessing the type object You can get the type object from a C++ class that has already been registered using: -.. code-block:: python +.. code-block:: cpp py::type T_py = py::type::of(); diff --git a/docs/advanced/embedding.rst b/docs/advanced/embedding.rst index dfdaad2d7..a435b8a75 100644 --- a/docs/advanced/embedding.rst +++ b/docs/advanced/embedding.rst @@ -122,6 +122,7 @@ embedding the interpreter. This makes it easy to import local Python files: """calc.py located in the working directory""" + def add(i, j): return i + j diff --git a/docs/advanced/functions.rst b/docs/advanced/functions.rst index a537cb65e..1178d0726 100644 --- a/docs/advanced/functions.rst +++ b/docs/advanced/functions.rst @@ -272,7 +272,7 @@ And used in Python as usual: .. code-block:: pycon - >>> print_dict({'foo': 123, 'bar': 'hello'}) + >>> print_dict({"foo": 123, "bar": "hello"}) key=foo, value=123 key=bar, value=hello @@ -377,10 +377,11 @@ argument in a function definition: def f(a, *, b): # a can be positional or via keyword; b must be via keyword pass + f(a=1, b=2) # good f(b=2, a=1) # good - f(1, b=2) # good - f(1, 2) # TypeError: f() takes 1 positional argument but 2 were given + f(1, b=2) # good + f(1, 2) # TypeError: f() takes 1 positional argument but 2 were given Pybind11 provides a ``py::kw_only`` object that allows you to implement the same behaviour by specifying the object between positional and keyword-only diff --git a/docs/advanced/pycpp/numpy.rst b/docs/advanced/pycpp/numpy.rst index cae068e91..30daeefff 100644 --- a/docs/advanced/pycpp/numpy.rst +++ b/docs/advanced/pycpp/numpy.rst @@ -258,8 +258,8 @@ by the compiler. The result is returned as a NumPy array of type .. code-block:: pycon - >>> x = np.array([[1, 3],[5, 7]]) - >>> y = np.array([[2, 4],[6, 8]]) + >>> x = np.array([[1, 3], [5, 7]]) + >>> y = np.array([[2, 4], [6, 8]]) >>> z = 3 >>> result = vectorized_func(x, y, z) @@ -403,7 +403,7 @@ In Python 2, the syntactic sugar ``...`` is not available, but the singleton .. code-block:: python - a = # a NumPy array + a = ... # a NumPy array b = a[0, ..., 0] The function ``py::ellipsis()`` function can be used to perform the same diff --git a/docs/advanced/pycpp/object.rst b/docs/advanced/pycpp/object.rst index 6fa8d0708..8bffb83e1 100644 --- a/docs/advanced/pycpp/object.rst +++ b/docs/advanced/pycpp/object.rst @@ -173,6 +173,7 @@ Keyword arguments are also supported. In Python, there is the usual call syntax: def f(number, say, to): ... # function code + f(1234, say="hello", to=some_instance) # keyword call in Python In C++, the same call can be made using: diff --git a/docs/advanced/pycpp/utilities.rst b/docs/advanced/pycpp/utilities.rst index 30429e84d..bf90a62f8 100644 --- a/docs/advanced/pycpp/utilities.rst +++ b/docs/advanced/pycpp/utilities.rst @@ -66,7 +66,7 @@ extra type, `py::scoped_estream_redirect `, is identica except for defaulting to ``std::cerr`` and ``sys.stderr``; this can be useful with `py::call_guard`, which allows multiple items, but uses the default constructor: -.. code-block:: py +.. code-block:: cpp // Alternative: Call single function using call guard m.def("noisy_func", &call_noisy_function, diff --git a/docs/advanced/smart_ptrs.rst b/docs/advanced/smart_ptrs.rst index da57748ca..5a2220109 100644 --- a/docs/advanced/smart_ptrs.rst +++ b/docs/advanced/smart_ptrs.rst @@ -77,6 +77,7 @@ segmentation fault). .. code-block:: python from example import Parent + print(Parent().get_child()) The problem is that ``Parent::get_child()`` returns a pointer to an instance of diff --git a/docs/benchmark.py b/docs/benchmark.py index 369470c81..f19079367 100644 --- a/docs/benchmark.py +++ b/docs/benchmark.py @@ -2,7 +2,6 @@ import datetime as dt import os import random -import time nfns = 4 # Functions per class nargs = 4 # Arguments per function diff --git a/docs/changelog.rst b/docs/changelog.rst index 8b9690caa..6bdf6a6e4 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1142,6 +1142,7 @@ v2.2.0 (August 31, 2017) from cpp_module import CppBase1, CppBase2 + class PyDerived(CppBase1, CppBase2): def __init__(self): CppBase1.__init__(self) # C++ bases must be initialized explicitly diff --git a/docs/classes.rst b/docs/classes.rst index f3610ef36..a4463e417 100644 --- a/docs/classes.rst +++ b/docs/classes.rst @@ -44,12 +44,12 @@ interactive Python session demonstrating this example is shown below: % python >>> import example - >>> p = example.Pet('Molly') + >>> p = example.Pet("Molly") >>> print(p) >>> p.getName() u'Molly' - >>> p.setName('Charly') + >>> p.setName("Charly") >>> p.getName() u'Charly' @@ -122,10 +122,10 @@ This makes it possible to write .. code-block:: pycon - >>> p = example.Pet('Molly') + >>> p = example.Pet("Molly") >>> p.name u'Molly' - >>> p.name = 'Charly' + >>> p.name = "Charly" >>> p.name u'Charly' @@ -174,10 +174,10 @@ Native Python classes can pick up new attributes dynamically: .. code-block:: pycon >>> class Pet: - ... name = 'Molly' + ... name = "Molly" ... >>> p = Pet() - >>> p.name = 'Charly' # overwrite existing + >>> p.name = "Charly" # overwrite existing >>> p.age = 2 # dynamically add a new attribute By default, classes exported from C++ do not support this and the only writable @@ -195,7 +195,7 @@ Trying to set any other attribute results in an error: .. code-block:: pycon >>> p = example.Pet() - >>> p.name = 'Charly' # OK, attribute defined in C++ + >>> p.name = "Charly" # OK, attribute defined in C++ >>> p.age = 2 # fail AttributeError: 'Pet' object has no attribute 'age' @@ -213,7 +213,7 @@ Now everything works as expected: .. code-block:: pycon >>> p = example.Pet() - >>> p.name = 'Charly' # OK, overwrite value in C++ + >>> p.name = "Charly" # OK, overwrite value in C++ >>> p.age = 2 # OK, dynamically add a new attribute >>> p.__dict__ # just like a native Python class {'age': 2} @@ -280,7 +280,7 @@ expose fields and methods of both types: .. code-block:: pycon - >>> p = example.Dog('Molly') + >>> p = example.Dog("Molly") >>> p.name u'Molly' >>> p.bark() @@ -486,7 +486,7 @@ typed enums. .. code-block:: pycon - >>> p = Pet('Lucy', Pet.Cat) + >>> p = Pet("Lucy", Pet.Cat) >>> p.type Kind.Cat >>> int(p.type) @@ -508,7 +508,7 @@ The ``name`` property returns the name of the enum value as a unicode string. .. code-block:: pycon - >>> p = Pet( "Lucy", Pet.Cat ) + >>> p = Pet("Lucy", Pet.Cat) >>> pet_type = p.type >>> pet_type Pet.Cat diff --git a/docs/compiling.rst b/docs/compiling.rst index eaf3270e0..073851048 100644 --- a/docs/compiling.rst +++ b/docs/compiling.rst @@ -42,10 +42,7 @@ An example of a ``setup.py`` using pybind11's helpers: ), ] - setup( - ..., - ext_modules=ext_modules - ) + setup(..., ext_modules=ext_modules) If you want to do an automatic search for the highest supported C++ standard, that is supported via a ``build_ext`` command override; it will only affect @@ -64,11 +61,7 @@ that is supported via a ``build_ext`` command override; it will only affect ), ] - setup( - ..., - cmdclass={"build_ext": build_ext}, - ext_modules=ext_modules - ) + setup(..., cmdclass={"build_ext": build_ext}, ext_modules=ext_modules) If you have single-file extension modules that are directly stored in the Python source tree (``foo.cpp`` in the same directory as where a ``foo.py`` diff --git a/docs/conf.py b/docs/conf.py index 458a86886..092e274e0 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -15,7 +15,6 @@ import os import re -import shlex import subprocess import sys from pathlib import Path diff --git a/docs/faq.rst b/docs/faq.rst index d6a048b06..e2f477b1f 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -54,7 +54,7 @@ provided by the caller -- in fact, it does nothing at all. .. code-block:: python def increment(i): - i += 1 # nope.. + i += 1 # nope.. pybind11 is also affected by such language-level conventions, which means that binding ``increment`` or ``increment_ptr`` will also create Python functions From b06a6f4f6294dd3550b1a2b6053421d8df145d3c Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Wed, 22 Sep 2021 17:41:56 -0400 Subject: [PATCH 2/5] feat: Slice allowing None with py::object or std::optional (#1101) * Adding nullptr slices Using example from #1095 Some fixes from @wjakob's review Stop clang-tidy from complaining New proposal for py::slice constructor Eric's suggested changes: simplify testing; shift def's * chore: drop MSVC pragma (hopefully unneeded) * Apply suggestions from code review --- include/pybind11/detail/common.h | 21 ++++++++++++++++++- include/pybind11/pytypes.h | 25 ++++++++++++++++++++--- include/pybind11/stl.h | 28 ++++++++------------------ tests/test_sequences_and_iterators.cpp | 17 ++++++++++++++++ tests/test_sequences_and_iterators.py | 11 ++++++++++ 5 files changed, 78 insertions(+), 24 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 8aeb79fb7..3bd84da57 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -161,7 +161,26 @@ // https://en.cppreference.com/w/c/chrono/localtime #if defined(__STDC_LIB_EXT1__) && !defined(__STDC_WANT_LIB_EXT1__) -# define __STDC_WANT_LIB_EXT1__ +# define __STDC_WANT_LIB_EXT1__ +#endif + +#ifdef __has_include +// std::optional (but including it in c++14 mode isn't allowed) +# if defined(PYBIND11_CPP17) && __has_include() +# define PYBIND11_HAS_OPTIONAL 1 +# endif +// std::experimental::optional (but not allowed in c++11 mode) +# if defined(PYBIND11_CPP14) && (__has_include() && \ + !__has_include()) +# define PYBIND11_HAS_EXP_OPTIONAL 1 +# endif +// std::variant +# if defined(PYBIND11_CPP17) && __has_include() +# define PYBIND11_HAS_VARIANT 1 +# endif +#elif defined(_MSC_VER) && defined(PYBIND11_CPP17) +# define PYBIND11_HAS_OPTIONAL 1 +# define PYBIND11_HAS_VARIANT 1 #endif #include diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index d1d3dcb05..383663b5e 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -14,6 +14,10 @@ #include #include +#if defined(PYBIND11_HAS_OPTIONAL) +# include +#endif + PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) /* A few forward declarations */ @@ -1345,11 +1349,20 @@ private: class slice : public object { public: PYBIND11_OBJECT_DEFAULT(slice, object, PySlice_Check) - slice(ssize_t start_, ssize_t stop_, ssize_t step_) { - int_ start(start_), stop(stop_), step(step_); + slice(handle start, handle stop, handle step) { m_ptr = PySlice_New(start.ptr(), stop.ptr(), step.ptr()); - if (!m_ptr) pybind11_fail("Could not allocate slice object!"); + if (!m_ptr) + pybind11_fail("Could not allocate slice object!"); } + +#ifdef PYBIND11_HAS_OPTIONAL + slice(std::optional start, std::optional stop, std::optional step) + : slice(index_to_object(start), index_to_object(stop), index_to_object(step)) {} +#else + slice(ssize_t start_, ssize_t stop_, ssize_t step_) + : slice(int_(start_), int_(stop_), int_(step_)) {} +#endif + bool compute(size_t length, size_t *start, size_t *stop, size_t *step, size_t *slicelength) const { return PySlice_GetIndicesEx((PYBIND11_SLICE_OBJECT *) m_ptr, @@ -1364,6 +1377,12 @@ public: stop, step, slicelength) == 0; } + +private: + template + static object index_to_object(T index) { + return index ? object(int_(*index)) : object(none()); + } }; class capsule : public object { diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 99b49d0e2..2c017b4fe 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -9,6 +9,7 @@ #pragma once +#include "detail/common.h" #include "pybind11.h" #include #include @@ -19,28 +20,15 @@ #include #include -#ifdef __has_include -// std::optional (but including it in c++14 mode isn't allowed) -# if defined(PYBIND11_CPP17) && __has_include() -# include -# define PYBIND11_HAS_OPTIONAL 1 -# endif -// std::experimental::optional (but not allowed in c++11 mode) -# if defined(PYBIND11_CPP14) && (__has_include() && \ - !__has_include()) -# include -# define PYBIND11_HAS_EXP_OPTIONAL 1 -# endif -// std::variant -# if defined(PYBIND11_CPP17) && __has_include() -# include -# define PYBIND11_HAS_VARIANT 1 -# endif -#elif defined(_MSC_VER) && defined(PYBIND11_CPP17) +// See `detail/common.h` for implementation of these guards. +#if defined(PYBIND11_HAS_OPTIONAL) # include +#elif defined(PYBIND11_HAS_EXP_OPTIONAL) +# include +#endif + +#if defined(PYBIND11_HAS_VARIANT) # include -# define PYBIND11_HAS_OPTIONAL 1 -# define PYBIND11_HAS_VARIANT 1 #endif PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) diff --git a/tests/test_sequences_and_iterators.cpp b/tests/test_sequences_and_iterators.cpp index 66d647262..72d96cb44 100644 --- a/tests/test_sequences_and_iterators.cpp +++ b/tests/test_sequences_and_iterators.cpp @@ -17,6 +17,11 @@ #include #include +#ifdef PYBIND11_HAS_OPTIONAL +#include +#endif // PYBIND11_HAS_OPTIONAL + + template class NonZeroIterator { const T* ptr_; @@ -117,6 +122,18 @@ TEST_SUBMODULE(sequences_and_iterators, m) { return std::make_tuple(istart, istop, istep); }); + m.def("make_forward_slice_size_t", []() { return py::slice(0, -1, 1); }); + m.def("make_reversed_slice_object", []() { return py::slice(py::none(), py::none(), py::int_(-1)); }); +#ifdef PYBIND11_HAS_OPTIONAL + m.attr("has_optional") = true; + m.def("make_reversed_slice_size_t_optional_verbose", []() { return py::slice(std::nullopt, std::nullopt, -1); }); + // Warning: The following spelling may still compile if optional<> is not present and give wrong answers. + // Please use with caution. + m.def("make_reversed_slice_size_t_optional", []() { return py::slice({}, {}, -1); }); +#else + m.attr("has_optional") = false; +#endif + // test_sequence class Sequence { public: diff --git a/tests/test_sequences_and_iterators.py b/tests/test_sequences_and_iterators.py index 2c73eff29..79689391a 100644 --- a/tests/test_sequences_and_iterators.py +++ b/tests/test_sequences_and_iterators.py @@ -16,6 +16,17 @@ def allclose(a_list, b_list, rel_tol=1e-05, abs_tol=0.0): ) +def test_slice_constructors(): + assert m.make_forward_slice_size_t() == slice(0, -1, 1) + assert m.make_reversed_slice_object() == slice(None, None, -1) + + +@pytest.mark.skipif(not m.has_optional, reason="no ") +def test_slice_constructors_explicit_optional(): + assert m.make_reversed_slice_size_t_optional() == slice(None, None, -1) + assert m.make_reversed_slice_size_t_optional_verbose() == slice(None, None, -1) + + def test_generalized_iterators(): assert list(m.IntPairs([(1, 2), (3, 4), (0, 5)]).nonzero()) == [(1, 2), (3, 4)] assert list(m.IntPairs([(1, 2), (2, 0), (0, 3), (4, 5)]).nonzero()) == [(1, 2)] From 1dc9a23caea407db3b7e148b1e9cb962a235b5ed Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 22 Sep 2021 22:38:04 -0400 Subject: [PATCH 3/5] chore(deps): bump jwlawson/actions-setup-cmake from 1.10 to 1.11 (#3294) Bumps [jwlawson/actions-setup-cmake](https://github.com/jwlawson/actions-setup-cmake) from 1.10 to 1.11. - [Release notes](https://github.com/jwlawson/actions-setup-cmake/releases) - [Commits](https://github.com/jwlawson/actions-setup-cmake/compare/v1.10...v1.11) --- updated-dependencies: - dependency-name: jwlawson/actions-setup-cmake dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 12 ++++++------ .github/workflows/configure.yml | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2348212f4..0b4968cc8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,7 +75,7 @@ jobs: run: brew install boost - name: Update CMake - uses: jwlawson/actions-setup-cmake@v1.10 + uses: jwlawson/actions-setup-cmake@v1.11 - name: Cache wheels if: runner.os == 'macOS' @@ -211,7 +211,7 @@ jobs: debug: ${{ matrix.python-debug }} - name: Update CMake - uses: jwlawson/actions-setup-cmake@v1.10 + uses: jwlawson/actions-setup-cmake@v1.11 - name: Valgrind cache if: matrix.valgrind @@ -463,7 +463,7 @@ jobs: run: python3 -m pip install --upgrade pip - name: Update CMake - uses: jwlawson/actions-setup-cmake@v1.10 + uses: jwlawson/actions-setup-cmake@v1.11 - name: Configure shell: bash @@ -756,7 +756,7 @@ jobs: architecture: x86 - name: Update CMake - uses: jwlawson/actions-setup-cmake@v1.10 + uses: jwlawson/actions-setup-cmake@v1.11 - name: Prepare MSVC uses: ilammy/msvc-dev-cmd@v1.9.0 @@ -802,7 +802,7 @@ jobs: python-version: ${{ matrix.python }} - name: Update CMake - uses: jwlawson/actions-setup-cmake@v1.10 + uses: jwlawson/actions-setup-cmake@v1.11 - name: Prepare MSVC uses: ilammy/msvc-dev-cmd@v1.9.0 @@ -856,7 +856,7 @@ jobs: python-version: ${{ matrix.python }} - name: Update CMake - uses: jwlawson/actions-setup-cmake@v1.10 + uses: jwlawson/actions-setup-cmake@v1.11 - name: Prepare env run: python -m pip install -r tests/requirements.txt --prefer-binary diff --git a/.github/workflows/configure.yml b/.github/workflows/configure.yml index d37fa3c2c..d60025e5c 100644 --- a/.github/workflows/configure.yml +++ b/.github/workflows/configure.yml @@ -55,7 +55,7 @@ jobs: # An action for adding a specific version of CMake: # https://github.com/jwlawson/actions-setup-cmake - name: Setup CMake ${{ matrix.cmake }} - uses: jwlawson/actions-setup-cmake@v1.10 + uses: jwlawson/actions-setup-cmake@v1.11 with: cmake-version: ${{ matrix.cmake }} From 2fa3fcfda5bb1aa1e8efc4d9cf90951e8055375e Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Wed, 22 Sep 2021 22:50:29 -0400 Subject: [PATCH 4/5] Revert "Add make_value_iterator (#3271)" This reverts commit ee0c5ee405e7a532410797687da28a20b89cd62b. --- docs/reference.rst | 3 - include/pybind11/pybind11.h | 118 +++++++------------------ tests/test_sequences_and_iterators.cpp | 58 ------------ tests/test_sequences_and_iterators.py | 29 ------ 4 files changed, 32 insertions(+), 176 deletions(-) diff --git a/docs/reference.rst b/docs/reference.rst index e64a03519..a678d41c8 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -63,9 +63,6 @@ Convenience functions converting to Python types .. doxygenfunction:: make_key_iterator(Iterator, Sentinel, Extra &&...) .. doxygenfunction:: make_key_iterator(Type &, Extra&&...) -.. doxygenfunction:: make_value_iterator(Iterator, Sentinel, Extra &&...) -.. doxygenfunction:: make_value_iterator(Type &, Extra&&...) - .. _extras: Passing extra arguments to ``def`` or ``class_`` diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index ac95b3a33..b8f5a6bae 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1955,52 +1955,25 @@ inline std::pair all_t return res; } -/* There are a large number of apparently unused template arguments because - * each combination requires a separate py::class_ registration. - */ -template +template struct iterator_state { Iterator it; Sentinel end; bool first_or_done; }; -// Note: these helpers take the iterator by non-const reference because some -// iterators in the wild can't be dereferenced when const. -template -struct iterator_access { - using result_type = decltype((*std::declval())); - // NOLINTNEXTLINE(readability-const-return-type) // PR #3263 - result_type operator()(Iterator &it) const { - return *it; - } -}; +PYBIND11_NAMESPACE_END(detail) -template -struct iterator_key_access { - using result_type = decltype(((*std::declval()).first)); - result_type operator()(Iterator &it) const { - return (*it).first; - } -}; - -template -struct iterator_value_access { - using result_type = decltype(((*std::declval()).second)); - result_type operator()(Iterator &it) const { - return (*it).second; - } -}; - -template ()), +#endif typename... Extra> -iterator make_iterator_impl(Iterator first, Sentinel last, Extra &&... extra) { - using state = detail::iterator_state; - // TODO: state captures only the types of Extra, not the values +iterator make_iterator(Iterator first, Sentinel last, Extra &&... extra) { + using state = detail::iterator_state; if (!detail::get_type_info(typeid(state), false)) { class_(handle(), "iterator", pybind11::module_local()) @@ -2014,7 +1987,7 @@ iterator make_iterator_impl(Iterator first, Sentinel last, Extra &&... extra) { s.first_or_done = true; throw stop_iteration(); } - return Access()(s.it); + return *s.it; // NOLINTNEXTLINE(readability-const-return-type) // PR #3263 }, std::forward(extra)..., Policy); } @@ -2022,55 +1995,35 @@ iterator make_iterator_impl(Iterator first, Sentinel last, Extra &&... extra) { return cast(state{first, last, true}); } -PYBIND11_NAMESPACE_END(detail) - -/// Makes a python iterator from a first and past-the-end C++ InputIterator. -template ::result_type, - typename... Extra> -iterator make_iterator(Iterator first, Sentinel last, Extra &&... extra) { - return detail::make_iterator_impl< - detail::iterator_access, - Policy, - Iterator, - Sentinel, - ValueType, - Extra...>(first, last, std::forward(extra)...); -} - -/// Makes a python iterator over the keys (`.first`) of a iterator over pairs from a +/// Makes an python iterator over the keys (`.first`) of a iterator over pairs from a /// first and past-the-end InputIterator. template ::result_type, +#ifndef DOXYGEN_SHOULD_SKIP_THIS // Issue in breathe 4.26.1 + typename KeyType = decltype((*std::declval()).first), +#endif typename... Extra> iterator make_key_iterator(Iterator first, Sentinel last, Extra &&...extra) { - return detail::make_iterator_impl< - detail::iterator_key_access, - Policy, - Iterator, - Sentinel, - KeyType, - Extra...>(first, last, std::forward(extra)...); -} + using state = detail::iterator_state; -/// Makes a python iterator over the values (`.second`) of a iterator over pairs from a -/// first and past-the-end InputIterator. -template ::result_type, - typename... Extra> -iterator make_value_iterator(Iterator first, Sentinel last, Extra &&...extra) { - return detail::make_iterator_impl< - detail::iterator_value_access, - Policy, Iterator, - Sentinel, - ValueType, - Extra...>(first, last, std::forward(extra)...); + if (!detail::get_type_info(typeid(state), false)) { + class_(handle(), "iterator", pybind11::module_local()) + .def("__iter__", [](state &s) -> state& { return s; }) + .def("__next__", [](state &s) -> detail::remove_cv_t { + if (!s.first_or_done) + ++s.it; + else + s.first_or_done = false; + if (s.it == s.end) { + s.first_or_done = true; + throw stop_iteration(); + } + return (*s.it).first; + }, std::forward(extra)..., Policy); + } + + return cast(state{first, last, true}); } /// Makes an iterator over values of an stl container or other container supporting @@ -2087,13 +2040,6 @@ template (std::begin(value), std::end(value), extra...); } -/// Makes an iterator over the values (`.second`) of a stl map-like container supporting -/// `std::begin()`/`std::end()` -template iterator make_value_iterator(Type &value, Extra&&... extra) { - return make_value_iterator(std::begin(value), std::end(value), extra...); -} - template void implicitly_convertible() { struct set_flag { bool &flag; diff --git a/tests/test_sequences_and_iterators.cpp b/tests/test_sequences_and_iterators.cpp index 72d96cb44..f4a78ae99 100644 --- a/tests/test_sequences_and_iterators.cpp +++ b/tests/test_sequences_and_iterators.cpp @@ -15,7 +15,6 @@ #include #include -#include #ifdef PYBIND11_HAS_OPTIONAL #include @@ -38,29 +37,6 @@ bool operator==(const NonZeroIterator>& it, const NonZeroSentine return !(*it).first || !(*it).second; } -class NonCopyableInt { -public: - explicit NonCopyableInt(int value) : value_(value) {} - NonCopyableInt(const NonCopyableInt &) = delete; - NonCopyableInt(NonCopyableInt &&other) noexcept : value_(other.value_) { - other.value_ = -1; // detect when an unwanted move occurs - } - NonCopyableInt &operator=(const NonCopyableInt &) = delete; - NonCopyableInt &operator=(NonCopyableInt &&other) noexcept { - value_ = other.value_; - other.value_ = -1; // detect when an unwanted move occurs - return *this; - } - int get() const { return value_; } - void set(int value) { value_ = value; } - ~NonCopyableInt() = default; -private: - int value_; -}; -using NonCopyableIntPair = std::pair; -PYBIND11_MAKE_OPAQUE(std::vector); -PYBIND11_MAKE_OPAQUE(std::vector); - template py::list test_random_access_iterator(PythonType x) { if (x.size() < 5) @@ -312,10 +288,6 @@ TEST_SUBMODULE(sequences_and_iterators, m) { .def( "items", [](const StringMap &map) { return py::make_iterator(map.begin(), map.end()); }, - py::keep_alive<0, 1>()) - .def( - "values", - [](const StringMap &map) { return py::make_value_iterator(map.begin(), map.end()); }, py::keep_alive<0, 1>()); // test_generalized_iterators @@ -334,38 +306,8 @@ TEST_SUBMODULE(sequences_and_iterators, m) { .def("nonzero_keys", [](const IntPairs& s) { return py::make_key_iterator(NonZeroIterator>(s.begin()), NonZeroSentinel()); }, py::keep_alive<0, 1>()) - .def("nonzero_values", [](const IntPairs& s) { - return py::make_value_iterator(NonZeroIterator>(s.begin()), NonZeroSentinel()); - }, py::keep_alive<0, 1>()) ; - // test_iterater_referencing - py::class_(m, "NonCopyableInt") - .def(py::init()) - .def("set", &NonCopyableInt::set) - .def("__int__", &NonCopyableInt::get) - ; - py::class_>(m, "VectorNonCopyableInt") - .def(py::init<>()) - .def("append", [](std::vector &vec, int value) { - vec.emplace_back(value); - }) - .def("__iter__", [](std::vector &vec) { - return py::make_iterator(vec.begin(), vec.end()); - }) - ; - py::class_>(m, "VectorNonCopyableIntPair") - .def(py::init<>()) - .def("append", [](std::vector &vec, const std::pair &value) { - vec.emplace_back(NonCopyableInt(value.first), NonCopyableInt(value.second)); - }) - .def("keys", [](std::vector &vec) { - return py::make_key_iterator(vec.begin(), vec.end()); - }) - .def("values", [](std::vector &vec) { - return py::make_value_iterator(vec.begin(), vec.end()); - }) - ; #if 0 // Obsolete: special data structure for exposing custom iterator types to python diff --git a/tests/test_sequences_and_iterators.py b/tests/test_sequences_and_iterators.py index 79689391a..44069fdd1 100644 --- a/tests/test_sequences_and_iterators.py +++ b/tests/test_sequences_and_iterators.py @@ -36,10 +36,6 @@ def test_generalized_iterators(): assert list(m.IntPairs([(1, 2), (2, 0), (0, 3), (4, 5)]).nonzero_keys()) == [1] assert list(m.IntPairs([(0, 3), (1, 2), (3, 4)]).nonzero_keys()) == [] - assert list(m.IntPairs([(1, 2), (3, 4), (0, 5)]).nonzero_values()) == [2, 4] - assert list(m.IntPairs([(1, 2), (2, 0), (0, 3), (4, 5)]).nonzero_values()) == [2] - assert list(m.IntPairs([(0, 3), (1, 2), (3, 4)]).nonzero_values()) == [] - # __next__ must continue to raise StopIteration it = m.IntPairs([(0, 0)]).nonzero() for _ in range(3): @@ -52,30 +48,6 @@ def test_generalized_iterators(): next(it) -def test_iterator_referencing(): - """Test that iterators reference rather than copy their referents.""" - vec = m.VectorNonCopyableInt() - vec.append(3) - vec.append(5) - assert [int(x) for x in vec] == [3, 5] - # Increment everything to make sure the referents can be mutated - for x in vec: - x.set(int(x) + 1) - assert [int(x) for x in vec] == [4, 6] - - vec = m.VectorNonCopyableIntPair() - vec.append([3, 4]) - vec.append([5, 7]) - assert [int(x) for x in vec.keys()] == [3, 5] - assert [int(x) for x in vec.values()] == [4, 7] - for x in vec.keys(): - x.set(int(x) + 1) - for x in vec.values(): - x.set(int(x) + 10) - assert [int(x) for x in vec.keys()] == [4, 6] - assert [int(x) for x in vec.values()] == [14, 17] - - def test_sliceable(): sliceable = m.Sliceable(100) assert sliceable[::] == (0, 100, 1) @@ -179,7 +151,6 @@ def test_map_iterator(): assert sm[k] == expected[k] for k, v in sm.items(): assert v == expected[k] - assert list(sm.values()) == [expected[k] for k in sm] it = iter(m.StringMap({})) for _ in range(3): # __next__ must continue to raise StopIteration From 5f46e47da8ef4c43252a934e9f461296140df1d2 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Thu, 23 Sep 2021 08:01:06 -0400 Subject: [PATCH 5/5] tests: check simple iteration of pairs (#3296) --- tests/test_sequences_and_iterators.cpp | 13 +++++++++++++ tests/test_sequences_and_iterators.py | 9 +++++++++ 2 files changed, 22 insertions(+) diff --git a/tests/test_sequences_and_iterators.cpp b/tests/test_sequences_and_iterators.cpp index f4a78ae99..16f8f5b23 100644 --- a/tests/test_sequences_and_iterators.cpp +++ b/tests/test_sequences_and_iterators.cpp @@ -295,6 +295,8 @@ TEST_SUBMODULE(sequences_and_iterators, m) { public: explicit IntPairs(std::vector> data) : data_(std::move(data)) {} const std::pair* begin() const { return data_.data(); } + // .end() only required for py::make_iterator(self) overload + const std::pair* end() const { return data_.data() + data_.size(); } private: std::vector> data_; }; @@ -306,6 +308,17 @@ TEST_SUBMODULE(sequences_and_iterators, m) { .def("nonzero_keys", [](const IntPairs& s) { return py::make_key_iterator(NonZeroIterator>(s.begin()), NonZeroSentinel()); }, py::keep_alive<0, 1>()) + .def("simple_iterator", [](IntPairs& self) { + return py::make_iterator(self); + }, py::keep_alive<0, 1>()) + .def("simple_keys", [](IntPairs& self) { + return py::make_key_iterator(self); + }, py::keep_alive<0, 1>()) + + // test iterator with keep_alive (doesn't work so not used at runtime, but tests compile) + .def("make_iterator_keep_alive", [](IntPairs& self) { + return py::make_iterator(self, py::keep_alive<0, 1>()); + }, py::keep_alive<0, 1>()) ; diff --git a/tests/test_sequences_and_iterators.py b/tests/test_sequences_and_iterators.py index 44069fdd1..902f4914c 100644 --- a/tests/test_sequences_and_iterators.py +++ b/tests/test_sequences_and_iterators.py @@ -48,6 +48,15 @@ def test_generalized_iterators(): next(it) +def test_generalized_iterators_simple(): + assert list(m.IntPairs([(1, 2), (3, 4), (0, 5)]).simple_iterator()) == [ + (1, 2), + (3, 4), + (0, 5), + ] + assert list(m.IntPairs([(1, 2), (3, 4), (0, 5)]).simple_keys()) == [1, 3, 0] + + def test_sliceable(): sliceable = m.Sliceable(100) assert sliceable[::] == (0, 100, 1)