Skip to content

fix: use Series.iloc for positional access in generate_join_column - #40936

Merged
eschutho merged 1 commit into
apache:masterfrom
eschutho:fix/series-getitem-iloc-helpers
Jun 11, 2026
Merged

eschutho merged 1 commit into
apache:masterfrom
eschutho:fix/series-getitem-iloc-helpers

Conversation

@eschutho

Copy link
Copy Markdown
Member

What

Silences a pandas FutureWarning that fires on every chart render using time-shift joins:

FutureWarning: Series.__getitem__ treating keys as positions is deprecated.
In a future version, integer keys will always be treated as labels
(consistent with DataFrame behavior). To access a value by position,
use `ser.iloc[pos]`
  value = row[column_index]

Source: superset/models/helpers.py — ExploreMixin.generate_join_column

Change

-        value = row[column_index]
+        value = row.iloc[column_index]

df.apply(..., axis=1) produces a pd.Series per row whose index labels are the DataFrame column names (always strings from SQL result sets). row[0] was being resolved positionally, which pandas now warns against. .iloc[column_index] is the explicit positional API and matches the original intent exactly.

No behavior change

row[0] and row.iloc[0] return the same value for any DataFrame whose columns are string-labeled (the only case that can reach this code path). This is a pure deprecation fix.

Test plan

  • Existing tests in tests/unit_tests/common/test_time_shifts.py cover generate_join_column via all four time grains and pass on this branch.
  • No new tests needed: the integer-label edge case cannot arise in production (all SQL result set columns are string-named).

Silences pandas FutureWarning: Series.__getitem__ treating integer keys
as positional is deprecated; use .iloc[pos] for explicit position access.
@bito-code-review

Copy link
Copy Markdown
Contributor

AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).

@eschutho
eschutho requested a review from rebenitez1802 June 10, 2026 16:07
@codecov

codecov Bot commented Jun 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.17%. Comparing base (2b58411) to head (556b453).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #40936   +/-   ##
=======================================
  Coverage   64.17%   64.17%           
=======================================
  Files        2654     2654           
  Lines      143763   143763           
  Branches    33161    33161           
=======================================
  Hits        92255    92255           
  Misses      49890    49890           
  Partials     1618     1618           
Flag Coverage Δ
hive 39.47% <0.00%> (ø)
mysql 58.21% <100.00%> (ø)
postgres 58.28% <100.00%> (ø)
presto 41.06% <0.00%> (ø)
python 59.75% <100.00%> (ø)
sqlite 57.90% <100.00%> (ø)
unit 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rebenitez1802 rebenitez1802 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.

LGTM

@eschutho
eschutho merged commit b0d7880 into apache:master Jun 11, 2026
59 of 60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants