Merge 'upstream/master' (8c0cd94) from previous merge-base (07e2259) #42

EricCousineau-TRI · 2020-06-21T17:38:45Z

upstream/master: pybind@8c0cd94 <- pybind@07e2259

Incorporates https://github.com/pybind/pybind11/releases/tag/v2.5.0

Process:

workspace: Update pybind11 fork to latest commit drake#13579 Submit PR to Drake using this branch. Ensure experimental CI passes
Review this PR.

Follows format of #34

This change is

Currently user specializations of the form template <typename itype> struct polymorphic_type_hook<itype, std::enable_if_t<...>> { ... }; will fail if itype is also polymorphic, because the existing specialization will also be enabled, which leads to 2 equally viable candidates. With this change, user provided specializations have higher priority than the built in specialization for polymorphic types.

…olymorphic.cpp. With this change, and cast.h as-is in master, test_tagbased_polymorphic.cpp fails to compile with the error message below. With the cast.h change in pull/2016, building and testing succeeds. cd pybind11/build/tests && /usr/bin/c++ -DPYBIND11_TEST_BOOST -DPYBIND11_TEST_EIGEN -Dpybind11_tests_EXPORTS -Ipybind11/include -I/usr/include/python3.7m -isystem /usr/include/eigen3 -Os -DNDEBUG -fPIC -fvisibility=hidden -std=c++2a -flto -fno-fat-lto-objects -Wall -Wextra -Wconversion -Wcast-qual -Wdeprecated -o CMakeFiles/pybind11_tests.dir/test_tagbased_polymorphic.cpp.o -c pybind11/tests/test_tagbased_polymorphic.cpp In file included from pybind11/include/pybind11/attr.h:13, from pybind11/include/pybind11/pybind11.h:44, from pybind11/tests/pybind11_tests.h:2, from pybind11/tests/test_tagbased_polymorphic.cpp:10: pybind11/include/pybind11/cast.h: In instantiation of ‘static std::pair<const void*, const pybind11::detail::type_info*> pybind11::detail::type_caster_base<type>::src_and_type(const itype*) [with type = Animal; pybind11::detail::type_caster_base<type>::itype = Animal]’: pybind11/include/pybind11/cast.h:906:31: required from ‘static pybind11::handle pybind11::detail::type_caster_base<type>::cast_holder(const itype*, const void*) [with type = Animal; pybind11::detail::type_caster_base<type>::itype = Animal]’ pybind11/include/pybind11/cast.h:1566:51: required from ‘static pybind11::handle pybind11::detail::move_only_holder_caster<type, holder_type>::cast(holder_type&&, pybind11::return_value_policy, pybind11::handle) [with type = Animal; holder_type = std::unique_ptr<Animal>]’ pybind11/include/pybind11/stl.h:175:69: required from ‘static pybind11::handle pybind11::detail::list_caster<Type, Value>::cast(T&&, pybind11::return_value_policy, pybind11::handle) [with T = std::vector<std::unique_ptr<Animal> >; Type = std::vector<std::unique_ptr<Animal> >; Value = std::unique_ptr<Animal>]’ pybind11/include/pybind11/pybind11.h:159:43: required from ‘void pybind11::cpp_function::initialize(Func&&, Return (*)(Args ...), const Extra& ...) [with Func = std::vector<std::unique_ptr<Animal> > (*&)(); Return = std::vector<std::unique_ptr<Animal> >; Args = {}; Extra = {pybind11::name, pybind11::scope, pybind11::sibling}]’ pybind11/include/pybind11/pybind11.h:64:9: required from ‘pybind11::cpp_function::cpp_function(Return (*)(Args ...), const Extra& ...) [with Return = std::vector<std::unique_ptr<Animal> >; Args = {}; Extra = {pybind11::name, pybind11::scope, pybind11::sibling}]’ pybind11/include/pybind11/pybind11.h:819:22: required from ‘pybind11::module& pybind11::module::def(const char*, Func&&, const Extra& ...) [with Func = std::vector<std::unique_ptr<Animal> > (*)(); Extra = {}]’ pybind11/tests/test_tagbased_polymorphic.cpp:141:36: required from here pybind11/include/pybind11/cast.h:880:61: error: ambiguous template instantiation for ‘struct pybind11::polymorphic_type_hook<Animal, void>’ const void *vsrc = polymorphic_type_hook<itype>::get(src, instance_type); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~ pybind11/include/pybind11/cast.h:844:8: note: candidates are: ‘template<class itype> struct pybind11::polymorphic_type_hook<itype, typename std::enable_if<std::is_polymorphic<_Tp>::value, void>::type> [with itype = Animal]’ struct polymorphic_type_hook<itype, detail::enable_if_t<std::is_polymorphic<itype>::value>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ pybind11/tests/test_tagbased_polymorphic.cpp:115:12: note: ‘template<class itype> struct pybind11::polymorphic_type_hook<itype, typename std::enable_if<std::is_base_of<Animal, itype>::value, void>::type> [with itype = Animal]’ struct polymorphic_type_hook<itype, detail::enable_if_t<std::is_base_of<Animal, itype>::value>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from pybind11/include/pybind11/attr.h:13, from pybind11/include/pybind11/pybind11.h:44, from pybind11/tests/pybind11_tests.h:2, from pybind11/tests/test_tagbased_polymorphic.cpp:10: pybind11/include/pybind11/cast.h:880:61: error: incomplete type ‘pybind11::polymorphic_type_hook<Animal, void>’ used in nested name specifier const void *vsrc = polymorphic_type_hook<itype>::get(src, instance_type); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~

This variable is a CMake community standard to set the C++ standard of a build. Document it in favor of the previous variable, which stays as a legacy flag for existing projects. https://cmake.org/cmake/help/v3.17/variable/CMAKE_CXX_STANDARD.html

__init__(self) cannot return values. According to https://stackoverflow.com/questions/2491819/how-to-return-a-value-from-init-in-python __new__(cls) should be used, which works.

…and set it

- Not currently supported on PyPy

This adds support for a `py::args_kw_only()` annotation that can be specified between `py::arg` annotations to indicate that any following arguments are keyword-only. This allows you to write: m.def("f", [](int a, int b) { /* ... */ }, py::arg("a"), py::args_kw_only(), py::arg("b")); and have it work like Python 3's: def f(a, *, b): # ... with respect to how `a` and `b` arguments are accepted (that is, `a` can be positional or by keyword; `b` can only be specified by keyword).

* Add AutoWIG to list of binding generators

This reverts commit 9ed8b44.

* Test on Python 3.9 beta * Pin Sphinx * Newer version of PyPy

Primarily for the ccmake curses GUI

Add warnings about extending STL

This change defines a new, portable macro PYBIND11_MAYBE_UNUSED to mark declarations as unused, and annotates the PYBIND11_MODULE entry point with this attribute. The purpose of this annotation is to facilitate dead code detection, which might otherwise consider the module entry point function dead, since it isn't otherwise used. (It is only used via FFI.)

# Conflicts: # .travis.yml # tests/test_class.cpp # tests/test_class.py

EricCousineau-TRI

+@sherm1 for review please! (as platform reviewer on Tuesday)

Please review this in the same way that Jeremy did on prior PR:
#34 (comment)

For full context, please review:
https://github.com/RobotLocomotion/pybind11/blob/drake/README_DRAKE.md#pulling-upstream-changes

Reviewable status: 0 of 40 files reviewed, all discussions resolved (waiting on @sherm1)

sherm1

This needs a feature review by someone who understands how this is supposed to work. I'm too far out of the loop to give this anything but a rubber stamp, which doesn't seem sufficient given the volume of changes. Maybe Jamie could provide an intelligent review? -@sherm1

Reviewable status: 0 of 40 files reviewed, all discussions resolved

EricCousineau-TRI · 2020-06-22T16:35:15Z

Sorry - what part of the instructions were unclear?
I would like to resolve this here, and I would like you to spend the time to be comfortable reviewing this change as platform reviewer.

I am happy to jump on a 10min VC with you to clarify the instructions.

sherm1 · 2020-06-22T16:50:55Z

I read all the material you referenced and it made me think a feature review is required. For example, in Jeremy's instructions it said "make sure the commit has the right parent". I don't know whether it does or doesn't, but a feature reviewer would presumably know what that means.

EricCousineau-TRI · 2020-06-22T23:17:17Z

"the merge commit has the correct parents" is basically just saying that the commit title is accurate, and is truly merging into robotlocomotion/drake rather than upstream/master.

I don't think it's that feature-based, just checking for local Git commit consistency - sorry for not clarifying that!

Yeah, I don't really expect you to check off the files.
It's just a question of "is this PR title correct" and "does CI pass in this repo and in Drake".

Can I still ask for your review in this light, just to make sure I don't make any foolish git errors?

sherm1

OK, I'll take a crack at it on Tuesday. +@sherm1

Reviewable status: 0 of 40 files reviewed, all discussions resolved (waiting on @sherm1)

EricCousineau-TRI · 2020-06-23T01:38:46Z

Thanks!!!

rpoyner-tri

@sherm1 feel free to flip this one to me if you'd prefer.

Reviewable status: 0 of 40 files reviewed, all discussions resolved (waiting on @sherm1)

sherm1

OK, thanks -- +@rpoyner-tri for platform review, please.
(I'll take a final look.)

Reviewable status: 0 of 40 files reviewed, all discussions resolved (waiting on @rpoyner-tri and @sherm1)

rpoyner-tri

Hashes and title are correct. Confirmed by pulling both fork, upstream, and PR branch locally and examining with good old gitk. Title checks out vs. provided instructions.
pending drake macOS CI evidence.

Reviewed 40 of 40 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @EricCousineau-TRI and @sherm1)

a discussion (no related file):
Where do I find evidence of Drake macOS build happiness for this branch?

sherm1 · 2020-06-23T17:02:30Z

All the CI evidence is in the companion PR RobotLocomotion/drake#13579 which alleges to be using this same pybind11 update. Take a look there and see if you are convinced. It does have Mac CI.

sherm1

Platform pending @rpoyner-tri's satisfaction with CI in the companion PR.

Reviewed 40 of 40 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @EricCousineau-TRI)

rpoyner-tri

Reviewable status: complete! all files reviewed, all discussions resolved

a discussion (no related file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

Where do I find evidence of Drake macOS build happiness for this branch?

Satisfied, per @sherm1 link to associated drake PR.

rpoyner-tri

Reviewable status: complete! all files reviewed, all discussions resolved

fwjavox and others added 30 commits January 17, 2020 01:16

stl_bind: add binding for std::vector::clear (pybind#2074)

e97c735

Fix the use of MSVC in an MSYS environment (pybind#2087)

4f72ef8

pybind11Tools.cmake: search for Python 3.9

bb9c91c

v2.5.0 release

3b1dbeb

begin working on next version

0234871

Utilize CMake's language standards abstraction when possible

d730fbc

Document CMAKE_CXX_STANDARD

6ebfc4b

This variable is a CMake community standard to set the C++ standard of a build. Document it in favor of the previous variable, which stays as a legacy flag for existing projects. https://cmake.org/cmake/help/v3.17/variable/CMAKE_CXX_STANDARD.html

Declare operator== and operator!= member functions const.

5088364

Change __init__(self) to __new__(cls)

9ed8b44

__init__(self) cannot return values. According to https://stackoverflow.com/questions/2491819/how-to-return-a-value-from-init-in-python __new__(cls) should be used, which works.

find library path to libclang.so via glob command in /usr/lib/llvm-* …

00c462d

…and set it

change set_path to set_file

9358e30

fix typo in sorted function call argument reverse

b14aeb7

Add is_final to disallow inheritance from Python

0dfffcf

- Not currently supported on PyPy

Fix compilation with clang-cl

03f9e4a

rename args_kw_only to kwonly

a86ac53

Adding method names to cpp_function constructor calls in enum_base

805c586

Propagate exceptions in sequence::size() (pybind#2076)

02c83db

Add AutoWIG to list of binding generators (pybind#1990)

2c4cd84

* Add AutoWIG to list of binding generators

Revert "Change __init__(self) to __new__(cls)"

a54eab9

This reverts commit 9ed8b44.

Fix CI, prepare test on Python 3.9 beta (pybind#2233)

a38e533

* Test on Python 3.9 beta * Pin Sphinx * Newer version of PyPy

pytypes.h: fix docs generation (pybind#2220)

a311813

cmake: Expose PYBIND11_TEST_OVERRIDE (pybind#2218)

2c30e0a

Primarily for the ccmake curses GUI

operators: Move hash check to before mutations, tweak whitespace

5309573

operators: Explicitly expose py::hash(py::self)

4e3d9fe

Add warnings about extending STL

Disable defining (v)snprintf as macro in modern Visual Studio

1817d21

Fix compiler error with MSVC 17 and CUDA 10.2

c776e9e

sizmailov and others added 9 commits June 10, 2020 13:36

Render full numpy numeric names (e.g. numpy.int32)

22b2504

Render py::iterator/py::iterable as Iterator/Iterable in docstrings

57070fb

Render pybind11::array as numpy.ndarray in docstrings

90d99b5

Render py::int_ as int in docstrings

4f1531c

Test py::iterable/py::iterator representation in docstrings

ab323e0

Fix setuptools record of headers

e107fc2

Fix docs typo

d96c345

ignore another type of visual studio project file

8c0cd94

Merge remote-tracking branch 'upstream/master' into drake

a1667c0

# Conflicts: # .travis.yml # tests/test_class.cpp # tests/test_class.py

EricCousineau-TRI mentioned this pull request Jun 21, 2020

workspace: Update pybind11 fork to latest commit RobotLocomotion/drake#13579

Merged

1 task

EricCousineau-TRI assigned sherm1 Jun 21, 2020

EricCousineau-TRI commented Jun 21, 2020

View reviewed changes

sherm1 removed their assignment Jun 22, 2020

sherm1 reviewed Jun 22, 2020

View reviewed changes

sherm1 self-assigned this Jun 23, 2020

sherm1 reviewed Jun 23, 2020

View reviewed changes

rpoyner-tri reviewed Jun 23, 2020

View reviewed changes

sherm1 assigned rpoyner-tri Jun 23, 2020

sherm1 reviewed Jun 23, 2020

View reviewed changes

rpoyner-tri requested changes Jun 23, 2020

View reviewed changes

sherm1 approved these changes Jun 23, 2020

View reviewed changes

rpoyner-tri approved these changes Jun 23, 2020

View reviewed changes

rpoyner-tri merged commit 5dfa2b3 into RobotLocomotion:drake Jun 23, 2020

EricCousineau-TRI mentioned this pull request Aug 3, 2020

Merge 'upstream/master' (1caf1d06) #43

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Merge 'upstream/master' (8c0cd94) from previous merge-base (07e2259) #42

Merge 'upstream/master' (8c0cd94) from previous merge-base (07e2259) #42

Uh oh!

EricCousineau-TRI commented Jun 21, 2020 •

edited

Loading

Uh oh!

EricCousineau-TRI left a comment

Uh oh!

sherm1 left a comment

Uh oh!

EricCousineau-TRI commented Jun 22, 2020

Uh oh!

sherm1 commented Jun 22, 2020

Uh oh!

EricCousineau-TRI commented Jun 22, 2020

Uh oh!

sherm1 left a comment

Uh oh!

EricCousineau-TRI commented Jun 23, 2020

Uh oh!

rpoyner-tri left a comment

Uh oh!

sherm1 left a comment

Uh oh!

rpoyner-tri left a comment

Uh oh!

sherm1 commented Jun 23, 2020

Uh oh!

sherm1 left a comment

Uh oh!

rpoyner-tri left a comment

Uh oh!

rpoyner-tri left a comment

Uh oh!

Uh oh!

Merge 'upstream/master' (8c0cd94) from previous merge-base (07e2259) #42

Merge 'upstream/master' (8c0cd94) from previous merge-base (07e2259) #42

Uh oh!

Conversation

EricCousineau-TRI commented Jun 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EricCousineau-TRI left a comment

Choose a reason for hiding this comment

Uh oh!

sherm1 left a comment

Choose a reason for hiding this comment

Uh oh!

EricCousineau-TRI commented Jun 22, 2020

Uh oh!

sherm1 commented Jun 22, 2020

Uh oh!

EricCousineau-TRI commented Jun 22, 2020

Uh oh!

sherm1 left a comment

Choose a reason for hiding this comment

Uh oh!

EricCousineau-TRI commented Jun 23, 2020

Uh oh!

rpoyner-tri left a comment

Choose a reason for hiding this comment

Uh oh!

sherm1 left a comment

Choose a reason for hiding this comment

Uh oh!

rpoyner-tri left a comment

Choose a reason for hiding this comment

Uh oh!

sherm1 commented Jun 23, 2020

Uh oh!

sherm1 left a comment

Choose a reason for hiding this comment

Uh oh!

rpoyner-tri left a comment

Choose a reason for hiding this comment

Uh oh!

rpoyner-tri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

EricCousineau-TRI commented Jun 21, 2020 •

edited

Loading