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

Give ability to bind SparseVector #70

Conversation

AlexandreAmice
Copy link

@AlexandreAmice AlexandreAmice commented Jun 13, 2024

I noticed that currently it is not possible to bind Drake methods which output a SparseVector because "there is no notion of compressed/uncompressed mode for a SparseVector".

This is a small fix attempting to fix this, but I am not familiar with this repository so don't know how to test this nor upgrade Drake with the fix.

cc @EricCousineau-TRI since you seem to be the maintainer


This change is Reviewable

Copy link
Collaborator

@EricCousineau-TRI EricCousineau-TRI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have example drake PR that shows this failing?

Also, if we'd want this to land, we'd want to add testing, ideally representative of what you encountered.

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

@jwnimmer-tri
Copy link

Drake policy re: this repository has changed over time. We have a goal of using vanilla upstream pybind11 (and eventually, nanobind). As such, we should strongly disprefer adding any patches here -- instead, we should add code into Drake that meets the need.

@jwnimmer-tri
Copy link

jwnimmer-tri commented Jan 7, 2025

As such, we should strongly disprefer adding any patches here -- instead, we should add code into Drake that meets the need.

Well, I suppose really there are two valid ways forward:

(1) Add utility function(s) to Drake's pydrake headers to solve the problem.

(2) Contribute a change upstream https://github.com/pybind/pybind11 to solve the problem.

In any case, new pull requests into our fork won't be accepted, so I'll close this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants