Skip to content

Conversation

@guan404ming
Copy link
Member

@guan404ming guan404ming commented Aug 6, 2025

Related PR

#53399
cc @eladkal

Why

  • Current read_pd_kwargs parameter is Pandas-specific, limiting extensibility
  • Need library-agnostic support for both Pandas and Polars DataFrames

How

  • Renamed read_pd_kwargsread_kwargs for library-agnostic naming
  • Added df_type parameter supporting "pandas" (default) and "polars"
  • Enhanced _partition_dataframe() to handle both DataFrame types
  • Maintained backward compatibility with deprecation warning

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg boring-cyborg bot added area:providers provider:amazon AWS/Amazon - related issues labels Aug 6, 2025
@guan404ming guan404ming marked this pull request as ready for review August 6, 2025 19:37
Copy link
Contributor

@eladkal eladkal left a comment

Choose a reason for hiding this comment

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

LGTM

@eladkal eladkal requested a review from vincbeck August 6, 2025 21:23
@guan404ming guan404ming force-pushed the sqltos3-polars branch 2 times, most recently from d1c00c9 to 534761a Compare August 7, 2025 07:45
@vincbeck vincbeck merged commit aa66153 into apache:main Aug 8, 2025
75 checks passed
@guan404ming guan404ming deleted the sqltos3-polars branch August 9, 2025 12:44
Comment on lines +258 to +259
if isinstance(df, pl.DataFrame):
df = df.to_pandas()
Copy link
Contributor

@eladkal eladkal Aug 17, 2025

Choose a reason for hiding this comment

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

@guan404ming Taking a closer look. Is this right?
This means that user who uses polars must also install pandas.

Copy link
Member Author

@guan404ming guan404ming Aug 17, 2025

Choose a reason for hiding this comment

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

You're right, to_pandas() still need pandas installed. That means it need re-implementation for this function for supporting two libs differently and I would open a PR for it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:amazon AWS/Amazon - related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants