Skip to content

fix: provide useful behavior of default py::slice #5620

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

Merged
merged 3 commits into from
Apr 16, 2025

Conversation

dyollb
Copy link
Contributor

@dyollb dyollb commented Apr 16, 2025

Description

Change the behavior of the default constructor of py::slice to be equivalent to vals[::]. I converted a large code base from boost::python to pybind11, and one of the stumbling blocks was this issue (see #5579).

Without this change, calling a[py::slice()] leads to an error, e.g. like this:

______________________________________________________________________________________ test_list_slicing _______________________________________________________________________________________

    def test_list_slicing():
        li = list(range(100))
        assert li[::2] == m.test_list_slicing(li)
>       assert li[::] == m.test_list_slicing_default(li)
E       SystemError: null argument to internal routine

li         = [0, 1, 2, 3, 4, 5, ...]

../../tests/test_pytypes.py:607: SystemError

Suggested changelog entry:

Change the behavior of the default constructor of `py::slice` to be equivalent to `::` in Python.

@henryiii henryiii changed the title Change behavior of default py::slice fix: provide useful behavior of default py::slice Apr 16, 2025
@dyollb
Copy link
Contributor Author

dyollb commented Apr 16, 2025

@henryiii I don't quite understand your comments - the code you are commenting on was not changed by my commit. Shall I commit your change suggestions?

@henryiii
Copy link
Collaborator

I commented the suggestion. Yes, it was just right next to your changes and might have contributed to your confusion in #5579 about the end point.

I think this looks good now!

@henryiii henryiii merged commit bc4a66d into pybind:master Apr 16, 2025
65 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs changelog Possibly needs a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants