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.

Sign up for GitHub

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

Jump to bottom

SNOW-1559348: Add Snowpark pandas TimedeltaIndex class #2160

Merged
merged 2 commits into from
Aug 28, 2024
Merged

SNOW-1559348: Add Snowpark pandas TimedeltaIndex class #2160

merged 2 commits into from
Aug 28, 2024

Conversation

sfc-gh-nkumar
Copy link
Contributor

@sfc-gh-nkumar sfc-gh-nkumar commented Aug 23, 2024 •
edited
Loading

Fixes SNOW-1559348

  • Add TimedeltaIndex class to provide lazy implementation.
  • Add documentation pages.
  • Only supports construction and raises not implemented error for timedelta index specific methods and attributes.
  • Also include type propagation fixes to query compiler methods set_index_from_series, set_index_from_columns and drop.

@sfc-gh-nkumar sfc-gh-nkumar force-pushed the nkumar-SNOW-1559348-timedelta-index branch from ad44696 to 9ea068a Compare August 23, 2024 21:18
@sfc-gh-nkumar sfc-gh-nkumar added the NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs label Aug 23, 2024
@sfc-gh-nkumar sfc-gh-nkumar marked this pull request as ready for review August 23, 2024 21:38
@sfc-gh-nkumar sfc-gh-nkumar requested a review from a team as a code owner August 23, 2024 21:38
@sfc-gh-nkumar sfc-gh-nkumar changed the title Nkumar snow 1559348 timedelta index SNOW-1559348: Add Snowpark pandas TimedeltaIndex class Aug 23, 2024
sfc-gh-azhan
sfc-gh-azhan reviewed Aug 23, 2024
View reviewed changes
Copy link
Collaborator

@sfc-gh-azhan sfc-gh-azhan left a comment

Choose a reason for hiding this comment

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

Are we going to add doc in a different PR?

tests/integ/modin/index/test_timedelta_index_methods.py
@sql_count_checker(query_count=3)
def test_timedelta_index_construction():
# create from native pandas timedelta index.
index = native_pd.TimedeltaIndex(["1 days", "2 days", "3 days"])
Copy link
Collaborator

Choose a reason for hiding this comment

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

just FYI, you can use pd.timedelta_range

@sfc-gh-nkumar sfc-gh-nkumar force-pushed the nkumar-SNOW-1559348-timedelta-index branch 7 times, most recently from 4cd66aa to a55932a Compare August 26, 2024 19:30
sfc-gh-nkrishna
sfc-gh-nkrishna reviewed Aug 27, 2024
View reviewed changes
Copy link
Contributor

@sfc-gh-nkrishna sfc-gh-nkrishna left a comment

Choose a reason for hiding this comment

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

LGTM overall, left two comments

CHANGELOG.md Outdated Show resolved Hide resolved
tests/integ/modin/index/test_timedelta_index_methods.py Show resolved Hide resolved
@sfc-gh-nkrishna
Copy link
Contributor

@sfc-gh-nkumar Looks like there are some Build Doc failures that need addressing

@sfc-gh-nkrishna sfc-gh-nkrishna mentioned this pull request Aug 27, 2024
Merged
@sfc-gh-nkumar sfc-gh-nkumar force-pushed the nkumar-SNOW-1559348-timedelta-index branch from a55932a to 1f9acd8 Compare August 27, 2024 17:23
sfc-gh-nkrishna
sfc-gh-nkrishna approved these changes Aug 27, 2024
View reviewed changes
Copy link
Contributor

@sfc-gh-nkrishna sfc-gh-nkrishna left a comment

Choose a reason for hiding this comment

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

Nice work! 🚢

@sfc-gh-nkumar sfc-gh-nkumar force-pushed the nkumar-SNOW-1559348-timedelta-index branch from 1f9acd8 to 1f95c5d Compare August 27, 2024 18:39
@sfc-gh-nkumar sfc-gh-nkumar force-pushed the nkumar-SNOW-1559348-timedelta-index branch from 1f95c5d to b3e49ac Compare August 27, 2024 22:48
@sfc-gh-nkumar sfc-gh-nkumar force-pushed the nkumar-SNOW-1559348-timedelta-index branch from b3e49ac to d0a63a4 Compare August 28, 2024 01:56
@sfc-gh-nkumar sfc-gh-nkumar merged commit 9876b4c into main Aug 28, 2024
33 of 35 checks passed
@sfc-gh-nkumar sfc-gh-nkumar deleted the nkumar-SNOW-1559348-timedelta-index branch August 28, 2024 17:41
@github-actions github-actions bot locked and limited conversation to collaborators Aug 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sfc-gh-nkrishna sfc-gh-nkrishna sfc-gh-nkrishna approved these changes

@sfc-gh-azhan sfc-gh-azhan sfc-gh-azhan approved these changes

@sfc-gh-rdurrani sfc-gh-rdurrani Awaiting requested review from sfc-gh-rdurrani sfc-gh-rdurrani is a code owner automatically assigned from snowflakedb/snowpark-pandas-api

@sfc-gh-dpetersohn sfc-gh-dpetersohn Awaiting requested review from sfc-gh-dpetersohn sfc-gh-dpetersohn is a code owner automatically assigned from snowflakedb/snowpark-pandas-api

@sfc-gh-vbudati sfc-gh-vbudati Awaiting requested review from sfc-gh-vbudati

Assignees
No one assigned
Labels
NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs snowpark-pandas
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfc-gh-nkumar @sfc-gh-nkrishna @sfc-gh-azhan