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 to slice a table using percentages #4135

Merged
merged 14 commits into from
Jul 14, 2023

Conversation

malhotrashivam
Copy link
Contributor

@malhotrashivam malhotrashivam commented Jul 5, 2023

Feature request, closes #3203

Change description: Added a new slicePct() method that takes percentages, similar to how slice() works with row indexes.

Documentation update: We need to add both java and python documentations, similar to the slice() method.
For Java, the syntax is table.slicePct(double startPercentInclusive, double endPercentExclusive)
For Python, the syntax is table.slice_pct(start_pct:float, end_pct:float)

The function returns a subset of table in the range [startPercentInclusive * sizeOfTable , endPercentExclusive * sizeOfTable). For example, for a table of size 10, slicePct(0.1, 0.7) will return a subset from the second row to the seventh row. Similarly, slicePct(0, 1) would return the entire table (because row positions run from 0 to size-1).
The percentage arguments must be in [0,1], otherwise the function returns an error.
Also, the method is not supported for blink tables and will give an error.

@malhotrashivam malhotrashivam added feature request New feature or request query engine core Core development tasks DocumentationNeeded NoReleaseNotesNeeded No release notes are needed. labels Jul 5, 2023
@malhotrashivam malhotrashivam added this to the July 2023 milestone Jul 5, 2023
@malhotrashivam malhotrashivam requested a review from chipkent as a code owner July 5, 2023 21:47
@malhotrashivam malhotrashivam self-assigned this Jul 5, 2023
@malhotrashivam malhotrashivam changed the title Added support to slice a table using percentages (WIP) Added support to slice a table using percentages Jul 5, 2023
@malhotrashivam malhotrashivam changed the title (WIP) Added support to slice a table using percentages Added support to slice a table using percentages Jul 6, 2023
Copy link
Contributor

@lbooker42 lbooker42 left a comment

Choose a reason for hiding this comment

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

Suggested test: Make sure that slices of a table when merged recreate exactly the original table.

Specifically, create a test where you create slices 0.0-0.25, 0.25-0.5, 0.5-0.75, 0.75-1.0 then verify all rows from the main table are in exactly one slice (no duplicates or missing rows).

lbooker42
lbooker42 previously approved these changes Jul 11, 2023
lbooker42
lbooker42 previously approved these changes Jul 11, 2023
jmao-denver
jmao-denver previously approved these changes Jul 14, 2023
chipkent
chipkent previously approved these changes Jul 14, 2023
@malhotrashivam malhotrashivam dismissed stale reviews from chipkent and jmao-denver via 1627902 July 14, 2023 18:35
@jmao-denver jmao-denver self-requested a review July 14, 2023 19:28
Copy link
Contributor

@jmao-denver jmao-denver left a comment

Choose a reason for hiding this comment

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

LGTM

@malhotrashivam malhotrashivam merged commit dca4fc5 into deephaven:main Jul 14, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jul 14, 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/2910
Reference: https://github.com/deephaven/deephaven.io/issues/2909

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.

Add Table.slicePct
6 participants