Skip to content

Add optional getColumn() to data frame#53

Merged
severo merged 8 commits intomasterfrom
accept-getcolumn-method
Feb 24, 2025
Merged

Add optional getColumn() to data frame#53
severo merged 8 commits intomasterfrom
accept-getcolumn-method

Conversation

@severo
Copy link
Copy Markdown
Contributor

@severo severo commented Feb 24, 2025

It will be used to compute the indexes of the sorted rows, to optimize the selection operations. See #34.

Reference discussion in #51

@severo severo changed the title use getColumn() to compute sorted indexes in sortableDataFrame Add optional getColumn() to data frame Feb 24, 2025
@severo severo force-pushed the accept-getcolumn-method branch from 6fdf6c6 to 5c991a1 Compare February 24, 2025 14:15
@severo severo force-pushed the accept-getcolumn-method branch from 5c991a1 to 917c1bc Compare February 24, 2025 15:11
@severo severo requested a review from platypii February 24, 2025 16:22
Comment thread src/dataframe.ts Outdated
*
* @returns Values in the column
*/
export type GetColumn = ({ column, start, end }: {column: string, start?: number, end?: number}) => Promise<any[]>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Eventually we may want to allow any[] OR TypedArrays as this will allow faster parsing from parquet. But I'd leave this as any[] for now, just making a note for the future.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, exactly. As I mentioned here: #51 (comment), I chose this for now because hyparquet does not provide a way to get the TypedArrays (if I read correctly)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment thread src/dataframe.ts Outdated
// if orderBy is provided, start and end are applied to the sorted rows
rows(args: RowsArgs): AsyncRow[]
// negative start and end are allowed
rows({ start, end, orderBy }: {start: number, end: number, orderBy?: string}): AsyncRow[]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The types here should probably have whitespace between the curly braces. Wonder if there's a typescript eslint rule.

Suggested change
rows({ start, end, orderBy }: {start: number, end: number, orderBy?: string}): AsyncRow[]
rows({ start, end, orderBy }: { start: number, end: number, orderBy?: string }): AsyncRow[]

Copy link
Copy Markdown
Contributor Author

@severo severo Feb 24, 2025

Choose a reason for hiding this comment

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

done.

Wonder if there's a typescript eslint rule.

Good question... I could look at it, maybe reopening hyparam/hyperparam-cli#35, if you want me to take time for this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not a priority

@severo severo merged commit 1fda119 into master Feb 24, 2025
@severo severo deleted the accept-getcolumn-method branch February 24, 2025 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants