Skip to content

SNOW-2331021: Wrap internal Series/DF creation#3768

Merged
sfc-gh-joshi merged 5 commits into
mainfrom
joshi-SNOW-2331021-wrap-index
Sep 11, 2025
Merged

SNOW-2331021: Wrap internal Series/DF creation#3768
sfc-gh-joshi merged 5 commits into
mainfrom
joshi-SNOW-2331021-wrap-index

Conversation

@sfc-gh-joshi

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

    Fixes SNOW-2331021

  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
    • If adding any arguments to public Snowpark APIs or creating new public Snowpark APIs, I acknowledge that I have ensured my changes include AST support. Follow the link for more information: AST Support Guidelines
  3. Please describe how your code solves the related issue.

Many internal functions create an instance of pd.Series or pd.DataFrame, then retrieve a query compiler from the object. With hybrid execution enabled, this query compiler is sometimes an instance of NativeQueryCompiler, which may break invariants when a SnowflakeQueryCompiler is expected instead. The new helper function new_snow_series and new_snow_df disable hybrid mode when creating a Series/DF for internal use.

This PR replaces constructor calls within the SFQC and indexing_overrides, which were responsible for the bug in the original ticket. Other constructor calls remain, but I have not yet triaged them.

This removes a few hundred test failures in modin_hybrid_integ_results.csv.

@sfc-gh-jkew sfc-gh-jkew left a comment

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.

Why did you need to rebuild the csv? Not complaining I just want to make sure that is not a regular occurance.

@sfc-gh-joshi

Copy link
Copy Markdown
Contributor Author

Why did you need to rebuild the csv? Not complaining I just want to make sure that is not a regular occurance.

Since this test was fixing a hybrid-specific bug, I wanted to see if the number of failures was reduced. It may help if we sort the DF on test name before exporting the CSV so the diff is less messy.

@sfc-gh-joshi sfc-gh-joshi added the NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs label Sep 11, 2025
@sfc-gh-jkew

Copy link
Copy Markdown
Contributor

Why did you need to rebuild the csv? Not complaining I just want to make sure that is not a regular occurance.

Since this test was fixing a hybrid-specific bug, I wanted to see if the number of failures was reduced. It may help if we sort the DF on test name before exporting the CSV so the diff is less messy.

Ahh, I see now in the description: " few hundred test failures" that's awesome.

@sfc-gh-joshi

Copy link
Copy Markdown
Contributor Author

Why did you need to rebuild the csv? Not complaining I just want to make sure that is not a regular occurance.

Since this test was fixing a hybrid-specific bug, I wanted to see if the number of failures was reduced. It may help if we sort the DF on test name before exporting the CSV so the diff is less messy.

Ahh, I see now in the description: " few hundred test failures" that's awesome.

Just to be clear, I didn't check which tests were fixed, I just compared the output of wc -l tests/integ/modin/modin_hybrid_integ_results.csv before (20706) and after (20191) this PR.

I just now sorted and dropped the position index of CSV, so diffs should be smaller in the future.

Comment thread tests/integ/modin/hybrid/test_switch_operations.py
@sfc-gh-joshi sfc-gh-joshi merged commit 1ea7657 into main Sep 11, 2025
27 of 28 checks passed
@sfc-gh-joshi sfc-gh-joshi deleted the joshi-SNOW-2331021-wrap-index branch September 11, 2025 21:33
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs snowpark-pandas

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants