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

SNOW-1707707: Add support for Index.to_numpy #2504

Merged
merged 9 commits into from
Oct 28, 2024

Conversation

sfc-gh-helmeleegy
Copy link
Contributor

@sfc-gh-helmeleegy sfc-gh-helmeleegy commented Oct 24, 2024

  1. Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes SNOW-1707707

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
      • If this test skips Local Testing mode, I'm requesting review from @snowflakedb/local-testing
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am adding new credentials
    • I am adding a new dependency
    • If this is a new feature/behavior, I'm adding the Local Testing parity changes.
    • I acknowledge that I have ensured my changes to be thread-safe. Follow the link for more information: Thread-safe Developer Guidelines
  3. Please describe how your code solves the related issue.

    Add support for Index.to_numpy.

@@ -2601,6 +2698,10 @@ def __array__(self, dtype: Any = None) -> np.ndarray:
"""
The array interface, return the values.
"""
# Ensure that the existing index dtype is preserved in the returned array
# if no other dtype is given.
if dtype is None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a pre-existing issue that was causing some test failures with the introduction of Index.to_numpy.

@sfc-gh-helmeleegy sfc-gh-helmeleegy force-pushed the helmeleegy-SNOW-1707707 branch 2 times, most recently from d8d190c to bb04347 Compare October 25, 2024 19:28
query_count=3 if isinstance(key, native_pd.Index) else 1, join_count=1
):
# df[boolean list-like key] is the same as df.loc[:, boolean list-like key]
if isinstance(key, native_pd.Index):
Copy link
Contributor Author

@sfc-gh-helmeleegy sfc-gh-helmeleegy Oct 25, 2024

Choose a reason for hiding this comment

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

There was no need to convert the native pandas index into a Snowpark pandas index when the df it's being passed to is actually a native pandas DataFrame. This was also causing test failures on the introduction of Index.to_numpy.

Copy link
Contributor

@sfc-gh-mvashishtha sfc-gh-mvashishtha left a comment

Choose a reason for hiding this comment

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

thanks!

@sfc-gh-helmeleegy sfc-gh-helmeleegy enabled auto-merge (squash) October 28, 2024 16:30
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, LGTM!

@sfc-gh-helmeleegy sfc-gh-helmeleegy merged commit 88ac441 into main Oct 28, 2024
39 checks passed
@sfc-gh-helmeleegy sfc-gh-helmeleegy deleted the helmeleegy-SNOW-1707707 branch October 28, 2024 16:31
@github-actions github-actions bot locked and limited conversation to collaborators Oct 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants