-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[workspace] Upgrade pybind11 to latest commit #20524
[workspace] Upgrade pybind11 to latest commit #20524
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least one assigned reviewer, labeled "do not merge" (waiting on @jwnimmer-tri)
a discussion (no related file):
Working: I'd like to have an automated test if possible to confirm this works as intended.
Will see if I can work 77c2e56 into this PR and confirm issue repro and the fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least one assigned reviewer, labeled "do not merge" (waiting on @jwnimmer-tri)
a discussion (no related file):
Previously, EricCousineau-TRI (Eric Cousineau) wrote…
Working: I'd like to have an automated test if possible to confirm this works as intended.
Will see if I can work 77c2e56 into this PR and confirm issue repro and the fix.
Ah, derp. I think we can't automate this, unless we have CI that actually has scipy
proper.
Will confirm this patch fixes #20519
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: needs platform reviewer assigned, needs at least one assigned reviewer, labeled "do not merge" (waiting on @jwnimmer-tri)
a discussion (no related file):
Previously, EricCousineau-TRI (Eric Cousineau) wrote…
Ah, derp. I think we can't automate this, unless we have CI that actually has
scipy
proper.Will confirm this patch fixes #20519
Done.
With command:
bazel run //bindings/pydrake/solvers:py/mathematicalprogram_test -- TestMathematicalProgram.test_linear_constraints
Confirmed it fails at af945db (#20519 + some changes)
Confirmed it works in c1acc16 (with this commit + some changes)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least one assigned reviewer, labeled "do not merge" (waiting on @jwnimmer-tri)
tools/workspace/pybind11/repository.bzl
line 11 at r1 (raw file):
# and if it has changed, then update the version number in the two # pybind11-*.cmake files in the current directory to match. _COMMIT = "2831b214d93cb1b0f35b77644a3056078025d2a3"
Needs bump to pybind11@drake
.
1ec1d52
to
09e9b7b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least one assigned reviewer (waiting on @jwnimmer-tri)
tools/workspace/pybind11/repository.bzl
line 11 at r1 (raw file):
Previously, EricCousineau-TRI (Eric Cousineau) wrote…
Needs bump to
pybind11@drake
.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all discussions resolved, LGTM from assignee EricCousineau-TRI(platform)
Closes #20516.
See RobotLocomotion/pybind11#69.
This change is