Skip to content

Conversation

@dcherian
Copy link
Contributor

@dcherian dcherian commented Mar 5, 2023

There was a bug in estimating the last index of the slice.
Index a range object instead.

dcherian added 2 commits March 4, 2023 22:15
There was a bug in estimating the last index of the slice.
Index a range object instead.

Closes pydata#6560
@dcherian dcherian changed the title Fix lazy slice rewriting. Fix lazy negative slice rewriting. Mar 5, 2023
Copy link
Collaborator

@headtr1ck headtr1ck left a comment

Choose a reason for hiding this comment

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

LGTM

stop = start + int((stop - start - 1) / step) * step + 1
start, stop = stop + 1, start + 1
return slice(start, stop, -step), slice(None, None, -1)
exact_stop = range(start, stop, step)[-1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a nice trick, have to remember that one :)

@dcherian dcherian added needs review plan to merge Final call for comments and removed needs review labels Mar 14, 2023
@dcherian
Copy link
Contributor Author

Thanks @headtr1ck I made the test smaller.

@headtr1ck
Copy link
Collaborator

Could this potentially affect performance?

@dcherian
Copy link
Contributor Author

Don't think so. I did add expliciit support for reversing a negative slice now. I think it's better?

%timeit range(0, 10**4)[-1]
%timeit range(0, 10**8)[-1]
374 ns ± 19.4 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)
372 ns ± 19.6 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)

@dcherian dcherian removed the plan to merge Final call for comments label Mar 16, 2023
@dcherian dcherian added the plan to merge Final call for comments label Mar 22, 2023
@dcherian dcherian merged commit 020b4c0 into pydata:main Mar 27, 2023
@dcherian dcherian deleted the fix-lazy-slicing branch March 27, 2023 21:05
dcherian added a commit to kmsquire/xarray that referenced this pull request Mar 29, 2023
* upstream/main:
  Save groupby codes after factorizing, pass to flox (pydata#7206)
  [skip-ci] Add compute to groupby benchmarks (pydata#7690)
  Delete built-in cfgrib backend (pydata#7670)
  Added a pronunciation guide to the word Xarray in the README.MD fil… (pydata#7677)
  boundarynorm fix (pydata#7553)
  Fix lazy negative slice rewriting. (pydata#7586)
  [pre-commit.ci] pre-commit autoupdate (pydata#7687)
  Adjust sidebar font colors (pydata#7674)
  Bump pypa/gh-action-pypi-publish from 1.8.1 to 1.8.3 (pydata#7682)
  Raise PermissionError when insufficient permissions (pydata#7629)
@dcherian dcherian mentioned this pull request Jun 30, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

io plan to merge Final call for comments topic-backends topic-indexing topic-zarr Related to zarr storage library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

netCDF + lazy backend: Error when sel is used with slice, reverse arrange

2 participants