Nanobind-only porting exploration

[rpoyner-tri]

Towards #21572.

---

This change is 

[rpoyner-tri]

+(status: defer ci)

@rpoyner-tri made 1 comment.
Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes.

[jwnimmer-tri]

@jwnimmer-tri reviewed 143 files and all commit messages, and made 8 comments.
Reviewable status: 8 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on rpoyner-tri).

---

a discussion (no related file):
Working

Get at least bazel test //bindings/pydrake/common/... passing under the python_binder=pybind11 flag, so that I can swap back and forth between pybind11 and nanobind on the same branch while testing dtype=object support.

---

MODULE.bazel line 149 at r1 (raw file):

    "eigen",
    "fmt",
    "nanobind",

Working

Need a final decision on whether to bury these in "internal", keep them here, or use the BCR flavor, or something in between.

---

systems/sensors/test_utilities/image_compare.cc line 26 at r1 (raw file):

  const int width = image.width();
  const int height = image.height();
  fmt::print((*os), "Image<k{}>(width={}, height={})", kPixelType, width,

nit What is this change doing here?

---

tools/skylark/pybind.bzl line 76 at r6 (raw file):

                "@drake//tools/workspace/nanobind",
                "@drake//tools/workspace/python:cc_headers",
                "@drake//tools/workspace/python:cc_libs",

Working

It's weird to list more than just nanobind here. It is deadwood, or is something wrong with nanobind?

---

bindings/pydrake/BUILD.bazel line 62 at r9 (raw file):

    hdrs = ["pydrake_pybind.h"],
    declare_installed_headers = 0,
    defines = select({

Working

These defines need to be provided to CMake users downstream, somehow.

---

bindings/pydrake/trajectories_py.cc line 102 at r10 (raw file):

  });
  cls->def("__setattr__", [](Class& self, py::str name, py::object value) {
    if (name.equal(py::str("breaks"))) {

Working

Land the py::str API changes on master to reduce diff burden.

#24578

---

tools/workspace/nanobind/package.BUILD.bazel line 10 at r1 (raw file):

package(default_visibility = ["//visibility:public"])

cc_library(

Working

The BCR module for this has a better build rule. We should probably mimic what it does (or just use it directly).

---

bindings/pydrake/common/test_utilities/autodiffutils_test_util_py.cc line 16 at r1 (raw file):

  // For testing implicit argument conversion.
  m.def("autodiff_scalar_pass_through", [](const AutoDiffXd& value) {
    return value;  // BR

nit The added BRs seem unnecessary? We have plenty of one-line lambdas in other bindings.

[jwnimmer-tri]

@jwnimmer-tri partially reviewed 19 files and all commit messages, made 3 comments, and resolved 2 discussions.
Reviewable status: 7 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on rpoyner-tri).

---

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Working

Get at least bazel test //bindings/pydrake/common/... passing under the python_binder=pybind11 flag, so that I can swap back and forth between pybind11 and nanobind on the same branch while testing dtype=object support.

This is basically done. I might widen the footprint more later, but for now it's sufficient.

---

bindings/pydrake/trajectories_py.cc line 102 at r10 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Working

Land the py::str API changes on master to reduce diff burden.

#24578

Done

---

tools/workspace/nanobind/patches/ndarray_extra_import.patch line 16 at r17 (raw file):

 
+// Hook for additional ndarray implicit conversion.
+NB_INLINE ndarray_handle *ndarray_extra_import(PyObject *,

Working

Instead of hooking nanobind to call into our type-caster of last resort, we could simply not include nanobind/eigen/dense.h and instead write our own type_caster that behaves the way we want it to. That would be better long-term, but I'm undecided about whether we do it before or after introducing nanobind support to our master branch.

[rpoyner-tri]

@rpoyner-tri made 3 comments.
Reviewable status: 7 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on jwnimmer-tri).

---

systems/sensors/test_utilities/image_compare.cc line 26 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

nit What is this change doing here?

I honestly don't recall. Feel free to drop it.

---

tools/skylark/pybind.bzl line 76 at r6 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Working

It's weird to list more than just nanobind here. It is deadwood, or is something wrong with nanobind?

I feel like I needed these at one point, but it may be misplaced or obsolete. If you delete them with no damage, then good riddance.

---

bindings/pydrake/common/test_utilities/autodiffutils_test_util_py.cc line 16 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

nit The added BRs seem unnecessary? We have plenty of one-line lambdas in other bindings.

yup, remove

[jwnimmer-tri]

@jwnimmer-tri reviewed 4 files and all commit messages, made 2 comments, and resolved 2 discussions.
Reviewable status: 5 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes.

---

tools/skylark/pybind.bzl line 76 at r6 (raw file):

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

I feel like I needed these at one point, but it may be misplaced or obsolete. If you delete them with no damage, then good riddance.

Yeah. Given wobbly test suite we have on the branch right now, I was going to first get all/most of the tests passing, and then circle back here to iterate on the spelling.

---

systems/sensors/test_utilities/image_compare.cc line 26 at r1 (raw file):

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

I honestly don't recall. Feel free to drop it.

Done. (Looking at it further, I wouldn't be surprised if it was merge conflict resolution fallout from the ostream deprecation journey.)

[rpoyner-tri]

@rpoyner-tri made 1 comment.
Reviewable status: 5 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on jwnimmer-tri).

---

tools/skylark/pybind.bzl line 76 at r6 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Yeah. Given wobbly test suite we have on the branch right now, I was going to first get all/most of the tests passing, and then circle back here to iterate on the spelling.

As a general matter, the build rules/code on the branch were the dead-minimum-effort to get things rolling. They definitely need a skeptical eye at some point.

[jwnimmer-tri]

@jwnimmer-tri partially reviewed 46 files and all commit messages.
Reviewable status: LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes.

[rpoyner-tri]

Closing (soon to be opened on a new PR), to reset review history.