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

Added support for (start<0 & stop>0) in slice #4076

Merged
merged 5 commits into from
Jul 12, 2023

Conversation

malhotrashivam
Copy link
Contributor

@malhotrashivam malhotrashivam commented Jun 27, 2023

Feature request, closes #3130

Change description: Earlier, an error was generated in this scenario. After this change, a subset of rows will be returned where start will be counted from the end of table (inclusively) and stop from the beginning. This change also fixes one bug where ADD_ONLY was incorrectly being propagated from parent to a child table in case of slice operation.

Documentation update: We need to update both the java and python documentation to reflect this change.
Earlier, if (start<0 & stop>0), an error was generated. Now, a subset of rows will be returned. For example, slice(-3, 5)
will return all rows starting from the third-last row to the fifth row of the table. If there are no rows between these positions, the function will return an empty table.
Note that index 5 corresponds to the sixth row in the table, so we are excluding the sixth row and ending the subset at the fifth row.

@malhotrashivam malhotrashivam added feature request New feature or request query engine core Core development tasks DocumentationNeeded NoReleaseNotesNeeded No release notes are needed. labels Jun 27, 2023
@malhotrashivam malhotrashivam added this to the June 2023 milestone Jun 27, 2023
@malhotrashivam malhotrashivam requested a review from lbooker42 June 27, 2023 00:19
@malhotrashivam malhotrashivam self-assigned this Jun 27, 2023
@malhotrashivam malhotrashivam changed the title Sm slice op Added support for (start<0 & stop>0) in slice Jun 27, 2023
Copy link
Member

@nbauernfeind nbauernfeind left a comment

Choose a reason for hiding this comment

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

It's probably worth having a pair of ticking/refreshing tests that ensure correctness. One that shrinks the source table to empty, and another that grows the source table until the slice is empty.

nbauernfeind
nbauernfeind previously approved these changes Jun 28, 2023
@malhotrashivam malhotrashivam merged commit c4141a2 into deephaven:main Jul 12, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jul 12, 2023
@deephaven-internal
Copy link
Contributor

Labels indicate documentation is required. Issues for documentation have been opened:

How-to: https://github.com/deephaven/deephaven.io/issues/2902
Reference: https://github.com/deephaven/deephaven.io/issues/2903

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core Core development tasks DocumentationNeeded feature request New feature or request NoReleaseNotesNeeded No release notes are needed. query engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broaden the types of indices that Table.slice supports
4 participants