Add support for Python slice objects for std::vector
#592
Conversation
|
Hi Carlos, I'll take care of reviewing this PR @jcarpent @nim65s. @cmastalli can you add a test for this feature? |
|
@ManifoldFR -- please note that apart from handling your comments, I included a Cmake command to enable us to run the Python tests individually. |
ManifoldFR
changed the title
std::vector
include/eigenpy/std-vector.hpp
Outdated
| static bp::object elem_ref(Container& c, index_type i) { | ||
| typename bp::to_python_indirect<value_type&, | ||
| bp::detail::make_reference_holder> conv; | ||
| return bp::object(bp::handle<>(conv(c[i]))); | ||
| } |
There was a problem hiding this comment.
Will this work with most types we bind? @jorisv
|
Hey @ManifoldFR and @jorisv! I have addressed your comments; however, I am unable to make changes to my commits as I am using a MacBook (i.e., the default filesystem is case-insensitive). In a nutshell, Git can’t represent the |
|
Hello @cmastalli, You're right about accelerate. We keep the old @ManifoldFR I see three options:
|
|
I say we go with the third option @jorisv. Accelerate isn't even something someone running a packaged version of an eigenpy release would be using - because the package would depend on Eigen 3.4 at most, which doesn't have Accelerate support! So breaking the include is a-ok in my book. |
|
Carlos can you fix the NPY002 error the ruff linter reports? |
jorisv
force-pushed
the
topic/slide-vector
branch
from
0873f3b to
75119b2
Compare
|
@ManifoldFR @cmastalli Accelerate.h has been fixed and I have rebase this PR on devel |
cmastalli
force-pushed
the
topic/slide-vector
branch
from
75119b2 to
a57d0df
Compare
Done! |
|
@jorisv and @ManifoldFR -- I have squashed a few commits and fixed the NPY002 error. It could be good to merge now! |
|
Ruff and clang-format are still complaining in the pre-commit run. Did you have pre-commit installed in your checked-out repo? |
|
Otherwise it will be good to merge |
Awesome! I didn't know that Eigenpy is using pre-commit. I just pushed a commit that runs |
The current
__getitem__only accepts integers, which means Eigenpy does not yet support Python slicing. So a slice object (xs[1:]) hits the "Invalid index type" path, as triggered in today's Crocoddyl pre-commit PR (see https://github.com/loco-3d/crocoddyl/actions/runs/17928428896/job/50980225223?pr=1433).This PR introduces a slice-aware
__getitem__. The basic strategy is to add an overload that acceptsboost::python::sliceand returns a Python list (with element references if you need mutability, mirroring our single-index path).I also included a copyright update, but I'm happy to remove it if you guys feel it's not justified.