SNOW-3923354 decouple pandas version from python connector and support pandas 3.x - #4312
SNOW-3923354 decouple pandas version from python connector and support pandas 3.x#4312sfc-gh-jzeng wants to merge 25 commits into
Conversation
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4312 +/- ##
==========================================
- Coverage 95.47% 89.34% -6.13%
==========================================
Files 171 170 -1
Lines 44749 44677 -72
Branches 7682 7686 +4
==========================================
- Hits 42723 39918 -2805
- Misses 1253 3627 +2374
- Partials 773 1132 +359 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
8caf817 to
166930f
Compare
…as 3 ColumnEmulator replaced pandas.Series._metadata (['_name']) instead of extending it. NDFrame.__finalize__ propagates only the intersection of the two _metadata lists, so every ColumnEmulator copy silently dropped _name and .name became None. The defect is pre-existing and version-independent -- it reproduces on pandas 2.3.1 in isolation. What changed is that pandas 3.0.5 added "grouper = grouper.copy(deep=False)" to Grouping.__init__, which forces that copy on every group-by, so the pivot and group-by family started failing. Upstream: pandas-dev/pandas#61491. Beyond pivot, a nulled .name silently corrupts mock_count_distinct (_functions.py:444 keys a TableEmulator on cols[i].name, collapsing every column onto a single None key with no exception) and _functions.py:962. No test covers either, which is why this is a product bug and not a pivot quirk. TableEmulator._metadata is deliberately left alone: pandas.DataFrame._metadata is empty, so it omits nothing, and adding "_name" there makes the DataFrame->Series __finalize__ intersection non-empty and clobbers correct column labels. tests/integ/scala --local_testing_mode on pandas 3.0.5: 22 failed/595 passed -> 9 failed/608 passed, zero regressions. tests/mock unchanged at 2/462. pandas 2.3.1 stays fully green at 617 passed.
DataFrame.sort_values re-wraps each sort column as pandas.Series(ndarray) before handing it to key= (pandas/core/frame.py; the line is identical in pandas 2 and 3). What changed is Series.__init__ inference: pandas 3 infers the dedicated str dtype for an object array of strings, and str's NA sentinel is nan. So a SQL NULL that is a real None in the frame reached custom_comparator as nan, `value_a is None` stopped firing, and the comparator fell through to a mixed float/str comparison.
pandas 3 infers the dedicated str dtype for an object array of strings and None, and str's NA sentinel is nan—so seven mock-layer sites that deliberately produced None had it silently converted on the way out of apply, combine, DataFrame.T.apply, iterrows, and a bare ColumnEmulator built from a list. Rebuild each result at explicit object dtype; gate the variant sink on isna_helper, since iterrows re-infers per row and has no upstream dtype to preserve.
A chained inplace replace mutates a temporary, which pandas 2 still wrote through to the parent while warning. pandas 3 is Copy-on-Write only, so the write is discarded and only announced via ChainedAssignmentError—a Warning subclass, so nothing raises. MERGE-INSERT columns omitted from the insert clause kept nan instead of None. Assign the result back.
…module__ pandas 3 re-homed read_sql from pandas.io.sql to the top-level pandas namespace, and code_generation routes "from X import Y" by Y.module, so the generated source legitimately changed. Product code is correct; only the expectation was stale. Interpolate the module rather than hard-coding the pandas-3 spelling, which would move the failure to the py310 job that still resolves pandas 2.
pandas 3 removed the integer-as-position fallback on Series.getitem, so iloc[0][0], dtypes[0] and row[0] from iterrows became label lookups that raise KeyError. Test-only: the vectorized UDFs run fine server-side, and product code's row[0] indexes a Snowpark Row, which is a tuple subclass. The accessor fix alone is not sufficient -- it unmasks two expectations that were only hidden because the KeyError fired first. VARCHAR-transported types (STRING, ARRAY, GEOGRAPHY, GEOMETRY, MAP) now arrive as str dtype rather than object, and pandas.Timestamp.module moved to the top-level namespace. Both accept either value, matching the spelling test_pandas_udf_input_types already uses, so the py310 job on pandas 2 keeps passing.
Pandas 3 renames two dtypes this test pins by string: uncast VARCHAR comes back as str instead of object, and the local-testing timestamp default resolution moved from ns to us. Labels only; values and instants are unchanged, so accept either spelling rather than branching on version. The live path already gets its expected timestamp dtype from pyarrow, so only local testing was hit. Verified on pandas 2.3.1 and 3.0.5, offline and against a live account.
write_pandas serializes to parquet, where a timedelta64 column becomes a duration Snowflake does not read back as a duration. The raw tick count lands in a NUMBER, so the unit is baked into the stored value. Pandas 2 inferred ns; pandas 3 infers us. The same timedelta(days=1) therefore started storing 86400000000 instead of 86400000000000 into the same column, with no warning. Nanoseconds is the contract the local testing emulator already enforces via Timedelta.value, and what every table written by an older client holds. The live path only matched that by accident, because ns was pandas' default. Normalizing at the single connector call site covers both write_pandas and create_dataframe(pdf). The to_pandas expectations go the other direction. The live path pins TIMESTAMP_NTZ to datetime64[ns] via pyarrow, while the emulator returns pandas' own default, so they cannot share a literal; the expectation is derived from local_testing_mode. String columns drop dtype=object, since plain inference now matches on both versions and both paths. Only the all-NULL column still needs an explicit dtype, because pd.Series([None]) still infers object on pandas 3.
MAP(STRING, INT) arrives as Arrow map<string, decimal128(38,0)>, so to_pandas() yields an object column of Decimal map values. Pandas 2 serializes that Decimal as a JSON number; pandas 3 emits a JSON string. Only one token changes: 1.0 becomes "1". Despite the test name, this is not a dtype change. The column is object on both versions, verified directly, so the dtype half of the expectation is left alone, as is the non-structured branch, whose values travel as VARIANT text and are byte-identical across versions. We never call DataFrame.to_json in product code; this only affects users who do.
Add `pandas_major_version` to `_internal/utils.py`
Split the unpinned local-testing job so both majors gate the PR.
write_arrow skipped the pandas write path, so a us duration was stored 1000x too small. Also stop calling the local-testing NULL fix pandas-3-only in the changelog.
These three still used Series.combine after the to_char fix, so a string NULL became nan and IS NULL missed the row.
Name the remaining semi-structured and geo string types, and stop saying the str/nan conversion cannot be disabled.
The helper-only Arrow test stays green if write_arrow skips normalize. Pin both write paths and the mock initcap(delimiters) branch.
The timedelta fix applies to any non-nanosecond column, so scoping the entry to pandas 3 told the affected pandas 2 users they were unaffected, and omitted that rows written earlier need correcting. The pandas cap for modin is declared by our own [modin] extra, not by modin itself.
The stored procedure suite uploads our test files to the server but runs them against the snowpark bundled in the Python UDF sandbox, which ships the released version rather than the branch build. A test module that imports a symbol added on this branch therefore cannot be collected at all, losing the whole file instead of skipping one assertion: ImportError: cannot import name 'pandas_major_version' from 'snowflake.snowpark._internal.utils' Jenkins trigger #103 caught this in PythonStoredProcBuildSnowfortTest. GitHub CI cannot, because there the installed snowpark is the branch itself and the import resolves. test_df_to_pandas.py carried the same import and was a latent second instance, masked by SNOW-3674599 skipping it for lack of pandas on Python 3.14. The constant in _internal/utils.py stays for product code, which always runs against its own tree.
166930f to
cf9a8eb
Compare
|
|
||
|
|
||
| # Use this for pandas 2 vs 3 branches instead of parsing __version__ inline. | ||
| pandas_major_version = ( |
There was a problem hiding this comment.
it appears that this is not imported anywhere in this PR, do we really need it?
There was a problem hiding this comment.
Thanks for the suggestion! This was imported but eventually fall back to the inline format after consideration. Already removed in the following commits.
| "pandas<3.0.0", | ||
| f"snowflake-connector-python{CONNECTOR_DEPENDENCY_VERSION}", | ||
| "pandas<4.0.0", | ||
| "pyarrow", |
There was a problem hiding this comment.
if I understand correctly, we add pyarrow here because we get rid of the [pandas] constraint?
If that is the case, do we need to align the pyarrow limit with connector? Like I remember they have a lower limit for pyarrow> xxx
There was a problem hiding this comment.
Yes, we previously relied on connector side which has pyarrow imported in its [pandas] extra.
Applied the same constraint mirroring with connector's side now.
The two test files that imported this symbol now derive the major version locally, because stored procedures run the server's released snowpark and importing it there fails collection with ImportError. That left the helper with no consumers. Keeping it is not neutral: the per-BCR plan bans version branches in product code wherever a version-agnostic spelling exists, so the helper has no intended caller by design, and anyone who imports it again reintroduces the stored-proc collection failure -- which only surfaces in the sproc suite, against a bundled older snowpark, where regular CI cannot see it. With this removed, _internal/utils.py is untouched by this branch.
… extra Before this branch the entry was snowflake-connector-python[pandas], and the extra suffix was the point: it is what pulled the connector's pandas and pyarrow in. Decoupling from that extra is the whole purpose of this change, which left the line resolving to exactly the specifier install_requires already declares via CONNECTOR_DEPENDENCY. An extra cannot be installed without the base package, so a [pandas] user always had the connector regardless. It also spelled the requirement out instead of reusing the constant, so it would drift if that constant's shape ever changed. The secure-local-storage extra keeps its full spelling, because that one does request an extra install_requires does not provide.
pyarrow.schema() builds a schema with no metadata, so rebuilding the table after casting a non-nanosecond duration column silently discarded whatever the caller's schema carried -- for a table produced from a pandas DataFrame that includes the pandas dtype and index information. Only the converting branch was affected; a table with no duration column returns early and was never touched. Found while reviewing this branch, in code the branch itself introduces, so there is nothing shipped to note.
Measured against the 1.53.0 and 1.54.0 sections, repo bullets run a median of 25 words with a maximum of 77. Two of ours exceeded that maximum at 78 and 84, so they are cut to 52 and 56 by dropping mechanism rather than consequence -- the data-correction warning and the affected-function list both stay, since those are what a reader acts on. The behaviour-change entry also asserted that DataFrame.collect() is unchanged. A "Behavior Changes" section describing something that did not change invites the reader to look for a relationship that is not there, and it read as contradicting the local-testing bug fix a few lines below, which is on the mock path. Removed, and the entry now states the dtype change as the cause of the NULL change rather than listing them as two separate facts.
Dropping connector[pandas] also dropped its pyarrow bounds, which we had been inheriting: a floor of >=14.0.1 since connector 4.5.0, plus a <24 ceiling on Python 3.14 added in 4.7.2. A bare pyarrow left both off. The floor mattered because our supported connector range starts at 3.17.0, whose own pyarrow requirement is unbounded, so nothing stopped a very old pyarrow from resolving. The ceiling mattered more: measured on CPython 3.14, snowpark[pandas] resolved pyarrow 25.0.1, and the connector then warned at import that this is a version it declares incompatible with its own Arrow code. With these bounds it resolves 23.0.1 and the warning is gone. pandas still resolves 3.0.5 on 3.12 through 3.14, so the point of the change is intact. These are deliberately a line-for-line mirror of the connector's own two pyarrow entries rather than a shorter equivalent spelling, so that diffing ours against theirs stays trivial when they move the bound -- they have moved it several times. The same marker-split idiom is already used two entries above for protobuf, for the same Python 3.14 reason. Only the pyarrow lines are mirrored. The connector's two pandas lines carry the <3.0.0 cap this change exists to lift, so those are deliberately not copied.
Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-3923354
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
The connector
[pandas]extra still pinspandas<3. This PR makes Snowpark's[pandas]extra declarepandas<4andpyarrowitself, so a client can install pandas 3. pandas 2 stays supported. A Snowpark bump does not upgrade an existing pandas 2 install.[modin]stays on pandas 2 (pandas<=2.4).Customer BCR on pandas 3:
to_pandas()/to_pandas_batches()use thestrdtype for text columns (VARCHAR,VARIANT,OBJECT,ARRAY,MAP, geo). SQL NULL in those columns comes back asnan, sovalue is Nonemisses. Usepandas.isna.collect()is unchanged.future.infer_string = Falsedoes not turn this off.write_pandas/write_arrow/create_dataframeconvert duration columns to ns before write. Snowflake stores a unit-less integer; the contract is ns. pandas 3 defaults tous.Local testing rebuilds columns as Python lists (
dtype=object) so SQL NULL staysNoneunder pandas 3str+ Copy-on-Write. Precommit local-testing has two jobs: pandas 2 and pandas 3.